From 18ef68e2adf5533339d985667b1ac78e49afa6a8 Mon Sep 17 00:00:00 2001 From: Jeroen van Bemmel Date: Thu, 3 Oct 2024 18:29:10 -0500 Subject: [PATCH 01/21] Clean branch with lag module changes --- docs/module/lag.md | 42 ++ netsim/ansible/tasks/frr/initial-clab.yml | 10 + netsim/ansible/templates/dhcp/cumulus.j2 | 6 +- netsim/ansible/templates/initial/cumulus.j2 | 5 +- netsim/ansible/templates/initial/frr.j2 | 1 + netsim/ansible/templates/lag/cumulus.j2 | 33 ++ netsim/ansible/templates/lag/frr.j2 | 33 ++ netsim/ansible/templates/vlan/cumulus.j2 | 9 +- netsim/augment/links.py | 15 +- netsim/defaults/attributes.yml | 2 +- netsim/devices/cumulus.yml | 3 + netsim/devices/frr.yml | 3 + netsim/modules/lag.py | 65 ++++ netsim/modules/lag.yml | 18 + netsim/modules/vlan.py | 2 +- netsim/modules/vlan.yml | 2 +- tests/errors/invalid-module.log | 4 +- tests/errors/link-invalid-type.log | 2 +- tests/errors/module-missing-prerequisite.log | 2 +- tests/errors/validate-list.log | 2 +- .../integration/lag/01-l3-lag-with-vlans.yml | 50 +++ tests/integration/lag/01-l3-lag.yml | 31 ++ tests/topology/expected/lag-l3-vlan-trunk.yml | 362 ++++++++++++++++++ tests/topology/expected/lag-l3.yml | 204 ++++++++++ tests/topology/input/lag-l3-vlan-trunk.yml | 24 ++ tests/topology/input/lag-l3.yml | 18 + 26 files changed, 925 insertions(+), 23 deletions(-) create mode 100644 docs/module/lag.md create mode 100644 netsim/ansible/templates/lag/cumulus.j2 create mode 100644 netsim/ansible/templates/lag/frr.j2 create mode 100644 netsim/modules/lag.py create mode 100644 netsim/modules/lag.yml create mode 100644 tests/integration/lag/01-l3-lag-with-vlans.yml create mode 100644 tests/integration/lag/01-l3-lag.yml create mode 100644 tests/topology/expected/lag-l3-vlan-trunk.yml create mode 100644 tests/topology/expected/lag-l3.yml create mode 100644 tests/topology/input/lag-l3-vlan-trunk.yml create mode 100644 tests/topology/input/lag-l3.yml diff --git a/docs/module/lag.md b/docs/module/lag.md new file mode 100644 index 0000000000..429b8b17a0 --- /dev/null +++ b/docs/module/lag.md @@ -0,0 +1,42 @@ +# Link Aggregation Group (LAG) Configuration Module + +This configuration module configures link bonding parameters, for LAGs between 2 devices (i.e. not MC-LAG) + +(lag-platform)= +LAG is currently supported on these platforms: + +| Operating system | lag | LACP off | LACP passive +| --------------------- | :-------: | :--------: | :----------: +| Cumulus Linux | ✅ | ✅ | ❌ +| FRR | ✅ | ✅ | ❌ + +## Parameters + +The following parameters can be set globally or per node/link/interface: + +* **lacp**: LACP protocol interval: "fast", "slow" or "off" + + Note that 'link down' is not easily detectable in a virtual environment with veth pairs, therefore it is strongly recommended + to enable LACP whenever possible + +* **lacp_mode**: "active" or "passive" + +The **lag.id** parameter can only be set at the link level; all links with the same lag.id value form a Link Aggregation Group. + +## Example + +To create a LAG consisting of 2 links between 2 devices: + +``` +module: [ lag ] + +nodes: [ r1, r2 ] + +links: +- r1: + r2: + lag.id: 1 +- r1: + r2: + lag.id: 1 +``` diff --git a/netsim/ansible/tasks/frr/initial-clab.yml b/netsim/ansible/tasks/frr/initial-clab.yml index 11a51ae568..b82ecd0240 100644 --- a/netsim/ansible/tasks/frr/initial-clab.yml +++ b/netsim/ansible/tasks/frr/initial-clab.yml @@ -5,6 +5,7 @@ modprobe vrf || echo "FAILED" become: true delegate_to: localhost + run_once: true tags: [ print_action, always ] register: modprobe_result ignore_errors: True @@ -14,5 +15,14 @@ set_fact: netlab_mgmt_vrf: False +- name: Load bonding kernel module + when: "'lag' in module|default([])" + shell: | + modprobe bonding + become: true + delegate_to: localhost + run_once: true + tags: [ print_action, always ] + - include_tasks: deploy-config.yml tags: [ print_action, always ] diff --git a/netsim/ansible/templates/dhcp/cumulus.j2 b/netsim/ansible/templates/dhcp/cumulus.j2 index 7df58b12d9..928ab4f740 100644 --- a/netsim/ansible/templates/dhcp/cumulus.j2 +++ b/netsim/ansible/templates/dhcp/cumulus.j2 @@ -2,7 +2,7 @@ # # Disable IPv6 RA on DHCPv6 client interfaces # -{% for l in interfaces if l.type in ['lan','p2p','stub'] and l.dhcp.client.ipv6 is defined %} +{% for l in interfaces if l.type in ['lan','p2p','stub','lag'] and l.dhcp.client.ipv6 is defined %} {% if loop.first %} echo "Disable IPv6 RA" cat >/tmp/config </etc/network/interfaces.d/12-dhcp.intf </etc/network/interfaces.d/11-physical.intf </etc/frr/vtysh.conf {% for l in interfaces if l.mtu is defined %} ip link set {{ l.ifname }} mtu {{ l.mtu }} {% endfor %} + # # Rest of initial configuration done through VTYSH # diff --git a/netsim/ansible/templates/lag/cumulus.j2 b/netsim/ansible/templates/lag/cumulus.j2 new file mode 100644 index 0000000000..43697e3e3c --- /dev/null +++ b/netsim/ansible/templates/lag/cumulus.j2 @@ -0,0 +1,33 @@ +#!/bin/bash +# +set -e + +echo "LAG: creating bond interface(s)" +# +# Create bond interface entry +# +{%- macro bond_interface(data) %} +auto {{ data.ifname }} +iface {{ data.ifname }} + pre-up ip link add {{ data.ifname }} type bond + bond-slaves {%- for i in interfaces if 'lag' in i and i.type=='p2p' and i.lag.id==data.lag.id %} {{ i.ifname }}{%- endfor %} +{% set _lacp = data.lag.lacp|default(lag.lacp) %} +{% if _lacp=='slow' %} + bond-lacp-rate slow +{% elif _lacp=='off' %} + bond-mode balance-xor +{% endif %} +{% endmacro %} + +cat >/etc/network/interfaces.d/20-bond.intf </etc/network/interfaces.d/51-bridge-interfaces.intf < list: link_intf = [] @@ -298,8 +298,8 @@ def add_node_interface(node: Box, ifdata: Box, defaults: Box) -> Box: if sys_mtu and node.mtu == ifdata.mtu: # .. is it equal to node MTU? ifdata.pop('mtu',None) # .... remove interface MTU on devices that support system MTU else: # Node MTU is defined, interface MTU is not - if not sys_mtu: # .. does the device support system MTU? - ifdata.mtu = node.mtu # .... no, copy node MTU to interface MTU + if not sys_mtu and ifdata.get('type',None)!='lag': # .. does the device support system MTU? + ifdata.mtu = node.mtu # .... no, copy node MTU to interface MTU unless it's a LAG node.interfaces.append(ifdata) @@ -381,6 +381,11 @@ def assign_link_prefix( addr_pools: Box, nodes: Box, link_path: str = 'links') -> Box: + + # Don't assign a prefix to physical links that are part of a lag + if 'lag' in link and link.get("type","")!="lag": + return data.get_empty_box() + if 'prefix' in link: # User specified a static link prefix pfx_list = addressing.parse_prefix(link.prefix,path=link_path) if log.debug_active('addr'): # pragma: no cover (debugging printout) @@ -851,7 +856,7 @@ def check_link_type(data: Box) -> bool: if link_type == 'loopback' and node_cnt != 1: log.error( - f'Looopback link {data._linkname} can have a single node attached\n... {data}', + f'Loopback link {data._linkname} can have a single node attached\n... {data}', log.IncorrectValue, 'links') return False @@ -1082,7 +1087,7 @@ def transform(link_list: typing.Optional[Box], defaults: Box, nodes: Box, pools: continue set_link_bridge_name(link,defaults) - link_default_pools = ['p2p','lan'] if link.type == 'p2p' else ['lan'] + link_default_pools = ['p2p','lan'] if link.type in ['p2p','lag'] else ['lan'] assign_link_prefix(link,link_default_pools,pools,nodes,link._linkname) copy_link_gateway(link,nodes) assign_interface_addresses(link,pools,nodes,defaults) diff --git a/netsim/defaults/attributes.yml b/netsim/defaults/attributes.yml index dfd0259c2d..0c0a2cabae 100644 --- a/netsim/defaults/attributes.yml +++ b/netsim/defaults/attributes.yml @@ -55,7 +55,7 @@ link: # Global link attributes _alt_types: [ bool, prefix_str, named_pfx ] role: id pool: id - type: { type: str, valid_values: [ lan, p2p, stub, loopback, tunnel, vlan_member ] } + type: { type: str, valid_values: [ lan, p2p, stub, loopback, tunnel, vlan_member, lag ] } unnumbered: bool interfaces: mtu: { type: int, min_value: 64, max_value: 65535 } diff --git a/netsim/devices/cumulus.yml b/netsim/devices/cumulus.yml index 059e283e22..66e0d942e6 100644 --- a/netsim/devices/cumulus.yml +++ b/netsim/devices/cumulus.yml @@ -3,6 +3,7 @@ description: Cumulus VX 4.x or 5.x configured without NVUE interface_name: swp{ifindex} loopback_interface_name: lo{ifindex if ifindex else ""} tunnel_interface_name: "tun{ifindex}" +lag_interface_name: "bond{ifindex}" mgmt_if: eth0 libvirt: image: CumulusCommunity/cumulus-vx:4.4.5 @@ -63,6 +64,8 @@ features: asymmetrical_irb: True gateway: protocol: [ anycast, vrrp ] + lag: + passive: False ospf: unnumbered: True import: [ bgp, ripv2, connected, vrf ] diff --git a/netsim/devices/frr.yml b/netsim/devices/frr.yml index 074a28760f..c8c552e987 100644 --- a/netsim/devices/frr.yml +++ b/netsim/devices/frr.yml @@ -4,6 +4,7 @@ interface_name: eth{ifindex} mgmt_if: eth0 loopback_interface_name: lo{ifindex if ifindex else ""} tunnel_interface_name: "tun{ifindex}" +lag_interface_name: "bond{ifindex}" routing: _rm_per_af: True group_vars: @@ -71,6 +72,8 @@ features: ipv4: true ipv6: true network: true + lag: + passive: False mpls: ldp: true vpn: diff --git a/netsim/modules/lag.py b/netsim/modules/lag.py new file mode 100644 index 0000000000..ab5a938764 --- /dev/null +++ b/netsim/modules/lag.py @@ -0,0 +1,65 @@ +import typing +import netaddr + +from box import Box +from . import _Module +from .. import data +from ..utils import log +from ..augment import devices + +class LAG(_Module): + + """ + link_pre_transform: Create virtual LAG links + """ + def link_pre_transform(self, link: Box, topology: Box) -> None: + if log.debug_active('vlan'): + print(f'LAG link_pre_transform for {link}') + if 'lag' in link and link.get('type',"")!="lag": + + # Lookup virtual LAG link with same id between same pair of nodes, create if not existing + vlag = None + for l in topology.links: + if 'lag' in l and l.get('type',"")=="lag" and l.lag.id == link.lag.id and l.interfaces==link.interfaces: + vlag = l + break + + if vlag is None: + vlag = data.get_box(link) + vlag.type = "lag" + vlag.linkindex = len(topology.links) + 1 + vlag._linkname = f"links[{vlag.linkindex}]" + vlag.interfaces = [ i for i in link.interfaces ] # Make a deep copy + vlag.pop('mtu',None) # Remove any MTU attribute + topology.links.append(vlag) + + if log.debug_active('vlan'): + print(f'LAG link_pre_transform created virtual link: {vlag}') + + # remove any VLAN attributes from original link + link.pop('vlan',None) + + """ + node_post_transform: Check for correct supported configuration of LAG + """ + def node_post_transform(self, node: Box, topology: Box) -> None: + features = devices.get_device_features(node,topology.defaults) + for i in node.interfaces: + # 1. Check if the interface is part of a LAG + if 'lag' in i: + if 'lag' not in features: + log.error( + f'Node {node.name} does not support LAG configured on {i.ifname}', + category=log.IncorrectAttr, + module='lag', + hint='lag') + + _type = i.get("type") + if _type=="lag": + continue + elif _type!="p2p": + log.error( + f'Node {node.name} has a LAG configured on {i.ifname} which is not a p2p link ({_type})', + category=log.IncorrectAttr, + module='lag', + hint='lag') diff --git a/netsim/modules/lag.yml b/netsim/modules/lag.yml new file mode 100644 index 0000000000..1542e86cbc --- /dev/null +++ b/netsim/modules/lag.yml @@ -0,0 +1,18 @@ +# LAG default settings and attributes +# +lacp: "fast" # Link Aggregation Control Protocol, standby signalling through link down not working +lacp_mode: "active" # At least 1 side must be active + +attributes: + global: + lacp: { type: str, valid_values: [ "off", "slow", "fast" ] } + lacp_mode: { type: str, valid_values: [ "passive", "active" ] } + node: + lacp: + lacp_mode: + link: # Most should be consistent across both interfaces on the link + id: { type: int, _required: True } + lacp: + lacp_mode: + interface: + lacp_mode: diff --git a/netsim/modules/vlan.py b/netsim/modules/vlan.py index 80c4cc85a6..eb7c8b307a 100644 --- a/netsim/modules/vlan.py +++ b/netsim/modules/vlan.py @@ -512,7 +512,7 @@ def create_vlan_link_data(init: typing.Union[Box,dict],vname: str, parent: typin return link_data """ -create_vlan_member_interface: Create interface data for a VLAN mamber link +create_vlan_member_interface: Create interface data for a VLAN member link Used by create_vlan_links and create_loopback_vlan_links """ diff --git a/netsim/modules/vlan.yml b/netsim/modules/vlan.yml index 1ef8e54482..9ec7068d2c 100644 --- a/netsim/modules/vlan.yml +++ b/netsim/modules/vlan.yml @@ -1,6 +1,6 @@ # VLAN default settings and attributes # ---- +transform_after: [ lag ] no_propagate: [ start_vlan_id, mode ] start_vlan_id: 1000 mode: irb diff --git a/tests/errors/invalid-module.log b/tests/errors/invalid-module.log index f3e75a55d6..f930454c43 100644 --- a/tests/errors/invalid-module.log +++ b/tests/errors/invalid-module.log @@ -1,5 +1,5 @@ IncorrectValue in topology: attribute nodes.r2.module has invalid value(s): whatever -... valid values are: bfd,bgp,dhcp,eigrp,evpn,gateway,isis,mpls,ospf,ripv2,routing,sr,srv6,vlan,vrf,vxlan +... valid values are: bfd,bgp,dhcp,eigrp,evpn,gateway,isis,lag,mpls,ospf,ripv2,routing,sr,srv6,vlan,vrf,vxlan IncorrectValue in topology: attribute module has invalid value(s): provider -... valid values are: bfd,bgp,dhcp,eigrp,evpn,gateway,isis,mpls,ospf,ripv2,routing,sr,srv6,vlan,vrf,vxlan +... valid values are: bfd,bgp,dhcp,eigrp,evpn,gateway,isis,lag,mpls,ospf,ripv2,routing,sr,srv6,vlan,vrf,vxlan Fatal error in netlab: Cannot proceed beyond this point due to errors, exiting diff --git a/tests/errors/link-invalid-type.log b/tests/errors/link-invalid-type.log index e1f37082a0..e47a24f608 100644 --- a/tests/errors/link-invalid-type.log +++ b/tests/errors/link-invalid-type.log @@ -1,4 +1,4 @@ IncorrectValue in links: attribute links[1].type has invalid value(s): wtf -... valid values are: lan,p2p,stub,loopback,tunnel,vlan_member +... valid values are: lan,p2p,stub,loopback,tunnel,vlan_member,lag ... use 'netlab show attributes link' to display valid attributes Fatal error in netlab: Cannot proceed beyond this point due to errors, exiting diff --git a/tests/errors/module-missing-prerequisite.log b/tests/errors/module-missing-prerequisite.log index 57497e761c..98c749d6b6 100644 --- a/tests/errors/module-missing-prerequisite.log +++ b/tests/errors/module-missing-prerequisite.log @@ -1,3 +1,3 @@ IncorrectValue in topology: attribute nodes.r1.module has invalid value(s): mody -... valid values are: bfd,bgp,dhcp,eigrp,evpn,gateway,isis,modx,mpls,ospf,ripv2,routing,sr,srv6,vlan,vrf,vxlan +... valid values are: bfd,bgp,dhcp,eigrp,evpn,gateway,isis,lag,modx,mpls,ospf,ripv2,routing,sr,srv6,vlan,vrf,vxlan Fatal error in netlab: Cannot proceed beyond this point due to errors, exiting diff --git a/tests/errors/validate-list.log b/tests/errors/validate-list.log index 0adeba9504..8bf3c25b25 100644 --- a/tests/errors/validate-list.log +++ b/tests/errors/validate-list.log @@ -1,5 +1,5 @@ IncorrectValue in groups: attribute groups.g1.module has invalid value(s): a -... valid values are: bfd,bgp,dhcp,eigrp,evpn,gateway,isis,mpls,ospf,ripv2,routing,sr,srv6,vlan,vrf,vxlan +... valid values are: bfd,bgp,dhcp,eigrp,evpn,gateway,isis,lag,mpls,ospf,ripv2,routing,sr,srv6,vlan,vrf,vxlan IncorrectType in groups: attribute 'groups.g2.module' must be a scalar or a list, found dictionary ... use 'netlab show attributes group' to display valid attributes IncorrectType in groups: attribute 'groups.g2.module' must be a scalar or a list, found dictionary diff --git a/tests/integration/lag/01-l3-lag-with-vlans.yml b/tests/integration/lag/01-l3-lag-with-vlans.yml new file mode 100644 index 0000000000..6d450de1fa --- /dev/null +++ b/tests/integration/lag/01-l3-lag-with-vlans.yml @@ -0,0 +1,50 @@ +--- +message: | + The devices under test are a pair of routers with a L3 LAG link with a trunk of VLANs between them + h1 and h2 should be able to ping each other, same applies for h3 and h4 +groups: + _auto_create: True + hosts: + members: [ h1, h2, h3, h4 ] + device: linux + provider: clab + switches: + members: [ r1,r2 ] + module: [lag,vlan] + +vlans: + v1: + v2: + +links: +- r1: + r2: + lag.id: 1 + vlan.trunk: [ v1, v2 ] +- r1: + r2: + lag.id: 1 +- r1: + vlan.access: v1 + h1: +- r2: + vlan.access: v1 + h2: +- r1: + vlan.access: v2 + h3: +- r2: + vlan.access: v2 + h4: + +validate: + ping12: + description: Pinging H2 from H1 on VLAN 1 + nodes: [ h1 ] + wait_msg: Waiting for STP to enable the ports + wait: 45 + plugin: ping('h2') + ping34: + description: Pinging H4 from H3 on VLAN 2 + nodes: [ h3 ] + plugin: ping('h4') \ No newline at end of file diff --git a/tests/integration/lag/01-l3-lag.yml b/tests/integration/lag/01-l3-lag.yml new file mode 100644 index 0000000000..1cb59c6332 --- /dev/null +++ b/tests/integration/lag/01-l3-lag.yml @@ -0,0 +1,31 @@ +--- +message: | + The devices under test are a pair of routers with a L3 LAG link between them + h1 and h2 should be able to ping each other +groups: + _auto_create: True + hosts: + members: [ h1, h2 ] + device: linux + provider: clab + switches: + members: [ r1,r2 ] + module: [lag,ospf] + +links: +- r1: + r2: + lag.id: 1 +- r1: + r2: + lag.id: 1 +- h1-r1 +- r2-h2 + +validate: + ping: + description: Pinging H2 from H1 + nodes: [ h1 ] + wait_msg: Waiting for STP to enable the ports + wait: 45 + plugin: ping('h2') \ No newline at end of file diff --git a/tests/topology/expected/lag-l3-vlan-trunk.yml b/tests/topology/expected/lag-l3-vlan-trunk.yml new file mode 100644 index 0000000000..a65dbef94e --- /dev/null +++ b/tests/topology/expected/lag-l3-vlan-trunk.yml @@ -0,0 +1,362 @@ +input: +- topology/input/lag-l3-vlan-trunk.yml +- package:topology-defaults.yml +lag: + lacp: fast + lacp_mode: active +links: +- interfaces: + - ifindex: 1 + ifname: eth1 + node: r1 + - ifindex: 1 + ifname: eth1 + node: r2 + lag: + id: 1 + linkindex: 1 + node_count: 2 + type: p2p +- interfaces: + - ifindex: 2 + ifname: eth2 + node: r1 + - ifindex: 2 + ifname: eth2 + node: r2 + lag: + id: 1 + linkindex: 2 + node_count: 2 + type: p2p +- bridge: input_3 + interfaces: + - ifindex: 30000 + ifname: bond0 + node: r1 + vlan: + trunk: + v1: {} + v2: {} + - ifindex: 30000 + ifname: bond0 + node: r2 + vlan: + trunk: + v1: {} + v2: {} + lag: + id: 1 + linkindex: 3 + node_count: 2 + prefix: {} + type: lag + vlan: + trunk: + v1: {} + v2: {} +module: +- lag +- vlan +name: input +nodes: + r1: + af: + ipv4: true + box: quay.io/frrouting/frr:10.0.1 + clab: + binds: + - clab_files/r1/daemons:/etc/frr/daemons + - clab_files/r1/hosts:/etc/hosts + config_templates: + - daemons:/etc/frr/daemons + - hosts:/etc/hosts + kind: linux + device: frr + hostname: clab-input-r1 + id: 1 + interfaces: + - ifindex: 1 + ifname: eth1 + lag: + id: 1 + linkindex: 1 + mtu: 1500 + name: r1 -> r2 + neighbors: + - ifname: eth1 + lag: + id: 1 + node: r2 + type: p2p + - ifindex: 2 + ifname: eth2 + lag: + id: 1 + linkindex: 2 + mtu: 1500 + name: r1 -> r2 + neighbors: + - ifname: eth2 + lag: + id: 1 + node: r2 + type: p2p + - ifindex: 30000 + ifname: bond0 + lag: + id: 1 + linkindex: 3 + name: r1 -> r2 + neighbors: + - ifname: bond0 + lag: + id: 1 + node: r2 + subif_index: 2 + type: lag + virtual_interface: true + - ifindex: 3 + ifname: bond0.1000 + parent_ifindex: 30000 + parent_ifname: bond0 + type: vlan_member + virtual_interface: true + vlan: + access: v1 + access_id: 1000 + - ifindex: 4 + ifname: bond0.1001 + parent_ifindex: 30000 + parent_ifname: bond0 + type: vlan_member + virtual_interface: true + vlan: + access: v2 + access_id: 1001 + - bridge_group: 1 + ifindex: 5 + ifname: vlan1000 + ipv4: 172.16.0.1/24 + name: VLAN v1 (1000) -> [r2] + neighbors: + - ifname: vlan1000 + ipv4: 172.16.0.2/24 + node: r2 + type: svi + virtual_interface: true + vlan: + mode: irb + - bridge_group: 2 + ifindex: 6 + ifname: vlan1001 + ipv4: 172.16.1.1/24 + name: VLAN v2 (1001) -> [r2] + neighbors: + - ifname: vlan1001 + ipv4: 172.16.1.2/24 + node: r2 + type: svi + virtual_interface: true + vlan: + mode: irb + lag: + lacp: fast + lacp_mode: active + loopback: + ifindex: 0 + ifname: lo + ipv4: 10.0.0.1/32 + neighbors: [] + type: loopback + virtual_interface: true + mgmt: + ifname: eth0 + ipv4: 192.168.121.101 + mac: 08:4f:a9:00:00:01 + module: + - lag + - vlan + mtu: 1500 + name: r1 + vlan: + max_bridge_group: 2 + vlans: + v1: + bridge_group: 1 + id: 1000 + mode: irb + prefix: + allocation: id_based + ipv4: 172.16.0.0/24 + v2: + bridge_group: 2 + id: 1001 + mode: irb + prefix: + allocation: id_based + ipv4: 172.16.1.0/24 + r2: + af: + ipv4: true + box: quay.io/frrouting/frr:10.0.1 + clab: + binds: + - clab_files/r2/daemons:/etc/frr/daemons + - clab_files/r2/hosts:/etc/hosts + config_templates: + - daemons:/etc/frr/daemons + - hosts:/etc/hosts + kind: linux + device: frr + hostname: clab-input-r2 + id: 2 + interfaces: + - ifindex: 1 + ifname: eth1 + lag: + id: 1 + linkindex: 1 + mtu: 1500 + name: r2 -> r1 + neighbors: + - ifname: eth1 + lag: + id: 1 + node: r1 + type: p2p + - ifindex: 2 + ifname: eth2 + lag: + id: 1 + linkindex: 2 + mtu: 1500 + name: r2 -> r1 + neighbors: + - ifname: eth2 + lag: + id: 1 + node: r1 + type: p2p + - ifindex: 30000 + ifname: bond0 + lag: + id: 1 + linkindex: 3 + name: r2 -> r1 + neighbors: + - ifname: bond0 + lag: + id: 1 + node: r1 + subif_index: 2 + type: lag + virtual_interface: true + - ifindex: 3 + ifname: bond0.1000 + parent_ifindex: 30000 + parent_ifname: bond0 + type: vlan_member + virtual_interface: true + vlan: + access: v1 + access_id: 1000 + - ifindex: 4 + ifname: bond0.1001 + parent_ifindex: 30000 + parent_ifname: bond0 + type: vlan_member + virtual_interface: true + vlan: + access: v2 + access_id: 1001 + - bridge_group: 1 + ifindex: 5 + ifname: vlan1000 + ipv4: 172.16.0.2/24 + name: VLAN v1 (1000) -> [r1] + neighbors: + - ifname: vlan1000 + ipv4: 172.16.0.1/24 + node: r1 + type: svi + virtual_interface: true + vlan: + mode: irb + - bridge_group: 2 + ifindex: 6 + ifname: vlan1001 + ipv4: 172.16.1.2/24 + name: VLAN v2 (1001) -> [r1] + neighbors: + - ifname: vlan1001 + ipv4: 172.16.1.1/24 + node: r1 + type: svi + virtual_interface: true + vlan: + mode: irb + lag: + lacp: fast + lacp_mode: active + loopback: + ifindex: 0 + ifname: lo + ipv4: 10.0.0.2/32 + neighbors: [] + type: loopback + virtual_interface: true + mgmt: + ifname: eth0 + ipv4: 192.168.121.102 + mac: 08:4f:a9:00:00:02 + module: + - lag + - vlan + mtu: 1500 + name: r2 + vlan: + max_bridge_group: 2 + vlans: + v1: + bridge_group: 1 + id: 1000 + mode: irb + prefix: + allocation: id_based + ipv4: 172.16.0.0/24 + v2: + bridge_group: 2 + id: 1001 + mode: irb + prefix: + allocation: id_based + ipv4: 172.16.1.0/24 +provider: clab +vlans: + v1: + host_count: 0 + id: 1000 + neighbors: + - ifname: vlan1000 + ipv4: 172.16.0.2/24 + node: r2 + - ifname: vlan1000 + ipv4: 172.16.0.1/24 + node: r1 + prefix: + allocation: id_based + ipv4: 172.16.0.0/24 + v2: + host_count: 0 + id: 1001 + neighbors: + - ifname: vlan1001 + ipv4: 172.16.1.2/24 + node: r2 + - ifname: vlan1001 + ipv4: 172.16.1.1/24 + node: r1 + prefix: + allocation: id_based + ipv4: 172.16.1.0/24 diff --git a/tests/topology/expected/lag-l3.yml b/tests/topology/expected/lag-l3.yml new file mode 100644 index 0000000000..af024d2fae --- /dev/null +++ b/tests/topology/expected/lag-l3.yml @@ -0,0 +1,204 @@ +input: +- topology/input/lag-l3.yml +- package:topology-defaults.yml +lag: + lacp: fast + lacp_mode: active +links: +- interfaces: + - ifindex: 1 + ifname: eth1 + node: r1 + - ifindex: 1 + ifname: eth1 + node: r2 + lag: + id: 1 + linkindex: 1 + mtu: 1600 + node_count: 2 + type: p2p +- interfaces: + - ifindex: 2 + ifname: eth2 + node: r1 + - ifindex: 2 + ifname: eth2 + node: r2 + lag: + id: 1 + linkindex: 2 + node_count: 2 + type: p2p +- bridge: input_3 + interfaces: + - ifindex: 30000 + ifname: bond0 + ipv4: 10.1.0.1/30 + node: r1 + - ifindex: 30000 + ifname: bond0 + ipv4: 10.1.0.2/30 + node: r2 + lag: + id: 1 + linkindex: 3 + node_count: 2 + prefix: + ipv4: 10.1.0.0/30 + type: lag +module: +- lag +name: input +nodes: + r1: + af: + ipv4: true + box: quay.io/frrouting/frr:10.0.1 + clab: + binds: + - clab_files/r1/daemons:/etc/frr/daemons + - clab_files/r1/hosts:/etc/hosts + config_templates: + - daemons:/etc/frr/daemons + - hosts:/etc/hosts + kind: linux + device: frr + hostname: clab-input-r1 + id: 1 + interfaces: + - ifindex: 1 + ifname: eth1 + lag: + id: 1 + linkindex: 1 + mtu: 1600 + name: r1 -> r2 + neighbors: + - ifname: eth1 + lag: + id: 1 + node: r2 + type: p2p + - ifindex: 2 + ifname: eth2 + lag: + id: 1 + linkindex: 2 + mtu: 1500 + name: r1 -> r2 + neighbors: + - ifname: eth2 + lag: + id: 1 + node: r2 + type: p2p + - ifindex: 30000 + ifname: bond0 + ipv4: 10.1.0.1/30 + lag: + id: 1 + linkindex: 3 + name: r1 -> r2 + neighbors: + - ifname: bond0 + ipv4: 10.1.0.2/30 + lag: + id: 1 + node: r2 + type: lag + virtual_interface: true + lag: + lacp: fast + lacp_mode: active + loopback: + ifindex: 0 + ifname: lo + ipv4: 10.0.0.1/32 + neighbors: [] + type: loopback + virtual_interface: true + mgmt: + ifname: eth0 + ipv4: 192.168.121.101 + mac: 08:4f:a9:00:00:01 + module: + - lag + mtu: 1500 + name: r1 + r2: + af: + ipv4: true + box: quay.io/frrouting/frr:10.0.1 + clab: + binds: + - clab_files/r2/daemons:/etc/frr/daemons + - clab_files/r2/hosts:/etc/hosts + config_templates: + - daemons:/etc/frr/daemons + - hosts:/etc/hosts + kind: linux + device: frr + hostname: clab-input-r2 + id: 2 + interfaces: + - ifindex: 1 + ifname: eth1 + lag: + id: 1 + linkindex: 1 + mtu: 1600 + name: r2 -> r1 + neighbors: + - ifname: eth1 + lag: + id: 1 + node: r1 + type: p2p + - ifindex: 2 + ifname: eth2 + lag: + id: 1 + linkindex: 2 + mtu: 1500 + name: r2 -> r1 + neighbors: + - ifname: eth2 + lag: + id: 1 + node: r1 + type: p2p + - ifindex: 30000 + ifname: bond0 + ipv4: 10.1.0.2/30 + lag: + id: 1 + linkindex: 3 + name: r2 -> r1 + neighbors: + - ifname: bond0 + ipv4: 10.1.0.1/30 + lag: + id: 1 + node: r1 + type: lag + virtual_interface: true + lag: + lacp: fast + lacp_mode: active + loopback: + ifindex: 0 + ifname: lo + ipv4: 10.0.0.2/32 + neighbors: [] + type: loopback + virtual_interface: true + mgmt: + ifname: eth0 + ipv4: 192.168.121.102 + mac: 08:4f:a9:00:00:02 + module: + - lag + mtu: 1500 + name: r2 +provider: clab diff --git a/tests/topology/input/lag-l3-vlan-trunk.yml b/tests/topology/input/lag-l3-vlan-trunk.yml new file mode 100644 index 0000000000..7cf347c605 --- /dev/null +++ b/tests/topology/input/lag-l3-vlan-trunk.yml @@ -0,0 +1,24 @@ +# +# Basic L3 LAG with VLANs example - ensure virtual 'lag' interfaces get created with L3 attributes moved from first physical link, +# and VLAN interfaces are associated with the correct elements +# + +defaults: + provider: clab + device: frr + +module: [ lag,vlan ] + +vlans: + v1: + v2: + +nodes: [ r1, r2 ] +links: +- r1: + r2: + lag.id: 1 + vlan.trunk: [v1,v2] +- r1: + r2: + lag.id: 1 diff --git a/tests/topology/input/lag-l3.yml b/tests/topology/input/lag-l3.yml new file mode 100644 index 0000000000..b891f7d912 --- /dev/null +++ b/tests/topology/input/lag-l3.yml @@ -0,0 +1,18 @@ +# +# Basic L3 LAG example - ensure virtual 'lag' interfaces get created with L3 attributes moved from first physical link +# + +defaults: + provider: clab + device: frr + +module: [ lag ] +nodes: [ r1, r2 ] +links: +- r1: + r2: + lag.id: 1 + mtu: 1600 # Test that mtu attribute is not applied to LAG interface +- r1: + r2: + lag.id: 1 From 56349af14865401cdae8609d79ad75e6713ff1fe Mon Sep 17 00:00:00 2001 From: Jeroen van Bemmel Date: Thu, 3 Oct 2024 19:13:17 -0500 Subject: [PATCH 02/21] Make sure the order of nodes doesn't matter --- netsim/modules/lag.py | 12 +++++++++++- tests/integration/lag/01-l3-lag.yml | 4 ++-- tests/topology/expected/lag-l3.yml | 4 ++-- tests/topology/input/lag-l3.yml | 4 ++-- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/netsim/modules/lag.py b/netsim/modules/lag.py index ab5a938764..a648c01506 100644 --- a/netsim/modules/lag.py +++ b/netsim/modules/lag.py @@ -7,6 +7,15 @@ from ..utils import log from ..augment import devices +# +# Checks if 2 lists have the same elements, independent of order +# +def same_list(l1,l2): + for l in l1: + if l not in l2: + return False + return len(l1)==len(l2) + class LAG(_Module): """ @@ -20,7 +29,8 @@ def link_pre_transform(self, link: Box, topology: Box) -> None: # Lookup virtual LAG link with same id between same pair of nodes, create if not existing vlag = None for l in topology.links: - if 'lag' in l and l.get('type',"")=="lag" and l.lag.id == link.lag.id and l.interfaces==link.interfaces: + if 'lag' in l and l.get('type',"")=="lag" and l.lag.id == link.lag.id \ + and same_list(l.interfaces,link.interfaces): vlag = l break diff --git a/tests/integration/lag/01-l3-lag.yml b/tests/integration/lag/01-l3-lag.yml index 1cb59c6332..e877482273 100644 --- a/tests/integration/lag/01-l3-lag.yml +++ b/tests/integration/lag/01-l3-lag.yml @@ -16,8 +16,8 @@ links: - r1: r2: lag.id: 1 -- r1: - r2: +- r2: + r1: lag.id: 1 - h1-r1 - r2-h2 diff --git a/tests/topology/expected/lag-l3.yml b/tests/topology/expected/lag-l3.yml index af024d2fae..809219ec64 100644 --- a/tests/topology/expected/lag-l3.yml +++ b/tests/topology/expected/lag-l3.yml @@ -21,10 +21,10 @@ links: - interfaces: - ifindex: 2 ifname: eth2 - node: r1 + node: r2 - ifindex: 2 ifname: eth2 - node: r2 + node: r1 lag: id: 1 linkindex: 2 diff --git a/tests/topology/input/lag-l3.yml b/tests/topology/input/lag-l3.yml index b891f7d912..fd5643ad97 100644 --- a/tests/topology/input/lag-l3.yml +++ b/tests/topology/input/lag-l3.yml @@ -13,6 +13,6 @@ links: r2: lag.id: 1 mtu: 1600 # Test that mtu attribute is not applied to LAG interface -- r1: - r2: +- r2: # Test that the order of declaration of nodes doesn't affect the outcome + r1: lag.id: 1 From 5ed9db29091241559d14c01f5c68a6f803fc973c Mon Sep 17 00:00:00 2001 From: Jeroen van Bemmel Date: Thu, 3 Oct 2024 19:26:52 -0500 Subject: [PATCH 03/21] Add annotation --- netsim/modules/lag.py | 6 +++--- tests/integration/lag/01-l3-lag-with-vlans.yml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/netsim/modules/lag.py b/netsim/modules/lag.py index a648c01506..7b14475fcc 100644 --- a/netsim/modules/lag.py +++ b/netsim/modules/lag.py @@ -1,7 +1,7 @@ import typing import netaddr -from box import Box +from box import Box, BoxList from . import _Module from .. import data from ..utils import log @@ -10,7 +10,7 @@ # # Checks if 2 lists have the same elements, independent of order # -def same_list(l1,l2): +def same_list(l1:BoxList,l2:BoxList) -> bool: for l in l1: if l not in l2: return False @@ -39,7 +39,7 @@ def link_pre_transform(self, link: Box, topology: Box) -> None: vlag.type = "lag" vlag.linkindex = len(topology.links) + 1 vlag._linkname = f"links[{vlag.linkindex}]" - vlag.interfaces = [ i for i in link.interfaces ] # Make a deep copy + vlag.interfaces = [ i for i in link.interfaces ] # Make a deep copy, could use a set? vlag.pop('mtu',None) # Remove any MTU attribute topology.links.append(vlag) diff --git a/tests/integration/lag/01-l3-lag-with-vlans.yml b/tests/integration/lag/01-l3-lag-with-vlans.yml index 6d450de1fa..428b080ba3 100644 --- a/tests/integration/lag/01-l3-lag-with-vlans.yml +++ b/tests/integration/lag/01-l3-lag-with-vlans.yml @@ -20,7 +20,7 @@ links: - r1: r2: lag.id: 1 - vlan.trunk: [ v1, v2 ] + vlan.trunk: [ v1, v2 ] # Note that any trunk MUST be declared on the first link in the LAG - r1: r2: lag.id: 1 From 6553823a5b068dcc0fbe0f128446dc96e86cac85 Mon Sep 17 00:00:00 2001 From: Jeroen van Bemmel Date: Fri, 4 Oct 2024 22:02:18 -0500 Subject: [PATCH 04/21] Reworked * New syntax * Addressed comments --- docs/module/lag.md | 24 +- netsim/ansible/tasks/frr/initial-clab.yml | 9 - netsim/ansible/templates/initial/frr.j2 | 2 +- netsim/ansible/templates/vlan/cumulus.j2 | 2 +- netsim/augment/links.py | 8 +- netsim/cli/__init__.py | 2 +- netsim/modules/lag.py | 99 ++--- netsim/modules/lag.yml | 1 + netsim/providers/clab.yml | 1 + tests/integration/lag/01-l3-lag.yml | 8 +- ...ith-vlans.yml => 02-l3-lag-with-vlans.yml} | 8 +- tests/topology/expected/lag-l3-vlan-trunk.yml | 341 +++++++++++++++--- tests/topology/expected/lag-l3.yml | 187 ++++++---- tests/topology/input/lag-l3-vlan-trunk.yml | 10 +- tests/topology/input/lag-l3.yml | 8 +- 15 files changed, 499 insertions(+), 211 deletions(-) rename tests/integration/lag/{01-l3-lag-with-vlans.yml => 02-l3-lag-with-vlans.yml} (87%) diff --git a/docs/module/lag.md b/docs/module/lag.md index 429b8b17a0..b185aca50b 100644 --- a/docs/module/lag.md +++ b/docs/module/lag.md @@ -6,7 +6,7 @@ This configuration module configures link bonding parameters, for LAGs between 2 LAG is currently supported on these platforms: | Operating system | lag | LACP off | LACP passive -| --------------------- | :-------: | :--------: | :----------: +| --------------------- | :-------: | :--------: | :----------: | Cumulus Linux | ✅ | ✅ | ❌ | FRR | ✅ | ✅ | ❌ @@ -21,7 +21,7 @@ The following parameters can be set globally or per node/link/interface: * **lacp_mode**: "active" or "passive" -The **lag.id** parameter can only be set at the link level; all links with the same lag.id value form a Link Aggregation Group. +By setting the **lag.id** parameter at the link level and defining **lag.members**, a *lag* type link is created with the given list or count of member links. ## Example @@ -36,7 +36,25 @@ links: - r1: r2: lag.id: 1 + lag.members: 2 +``` +Additional parameters such as vlan trunks, OSPF cost, etc. can be applied to such *lag* type links. + +In case additional attributes are required for the member links, the following syntax can also be used +``` +links: - r1: r2: - lag.id: 1 + lag: + id: 1 + members: + - r1: + ifindex: 49 # Use 100G links 1/1/49 and 1/1/50 + r2: + ifindex: 49 + - r1: + ifindex: 50 + r2: + ifindex: 50 ``` +Naturally, the links in lag.members can only use nodes associated with the lag link \ No newline at end of file diff --git a/netsim/ansible/tasks/frr/initial-clab.yml b/netsim/ansible/tasks/frr/initial-clab.yml index b82ecd0240..23fe209fef 100644 --- a/netsim/ansible/tasks/frr/initial-clab.yml +++ b/netsim/ansible/tasks/frr/initial-clab.yml @@ -15,14 +15,5 @@ set_fact: netlab_mgmt_vrf: False -- name: Load bonding kernel module - when: "'lag' in module|default([])" - shell: | - modprobe bonding - become: true - delegate_to: localhost - run_once: true - tags: [ print_action, always ] - - include_tasks: deploy-config.yml tags: [ print_action, always ] diff --git a/netsim/ansible/templates/initial/frr.j2 b/netsim/ansible/templates/initial/frr.j2 index d652396ff4..258e78c4f3 100644 --- a/netsim/ansible/templates/initial/frr.j2 +++ b/netsim/ansible/templates/initial/frr.j2 @@ -99,7 +99,7 @@ ip link set {{ l.ifname }} up echo "service integrated-vtysh-config" >/etc/frr/vtysh.conf # # Set Ethernet interface MTU -{% for l in interfaces if l.mtu is defined %} +{% for l in interfaces if l.mtu is defined and l.get('type',"")!='lag' %} ip link set {{ l.ifname }} mtu {{ l.mtu }} {% endfor %} diff --git a/netsim/ansible/templates/vlan/cumulus.j2 b/netsim/ansible/templates/vlan/cumulus.j2 index 48134a4d45..f14dcede6f 100644 --- a/netsim/ansible/templates/vlan/cumulus.j2 +++ b/netsim/ansible/templates/vlan/cumulus.j2 @@ -34,7 +34,7 @@ cat >/etc/network/interfaces.d/51-bridge-interfaces.intf < Box: if sys_mtu and node.mtu == ifdata.mtu: # .. is it equal to node MTU? ifdata.pop('mtu',None) # .... remove interface MTU on devices that support system MTU else: # Node MTU is defined, interface MTU is not - if not sys_mtu and ifdata.get('type',None)!='lag': # .. does the device support system MTU? - ifdata.mtu = node.mtu # .... no, copy node MTU to interface MTU unless it's a LAG + if not sys_mtu: # .. does the device support system MTU? + ifdata.mtu = node.mtu # .... no, copy node MTU to interface MTU node.interfaces.append(ifdata) @@ -381,10 +381,6 @@ def assign_link_prefix( addr_pools: Box, nodes: Box, link_path: str = 'links') -> Box: - - # Don't assign a prefix to physical links that are part of a lag - if 'lag' in link and link.get("type","")!="lag": - return data.get_empty_box() if 'prefix' in link: # User specified a static link prefix pfx_list = addressing.parse_prefix(link.prefix,path=link_path) diff --git a/netsim/cli/__init__.py b/netsim/cli/__init__.py index 8b89cb31e8..ed234bd657 100755 --- a/netsim/cli/__init__.py +++ b/netsim/cli/__init__.py @@ -27,7 +27,7 @@ def parser_add_debug(parser: argparse.ArgumentParser) -> None: choices=sorted([ 'all','addr','cli','links','libvirt','clab','modules','plugin','template', 'vlan','vrf','quirks','validate','addressing','groups','status', - 'external','defaults']), + 'external','defaults','lag']), help=argparse.SUPPRESS) parser.add_argument('--test', dest='test', action='store',nargs='*', choices=['errors'], diff --git a/netsim/modules/lag.py b/netsim/modules/lag.py index 7b14475fcc..8f1a7f6280 100644 --- a/netsim/modules/lag.py +++ b/netsim/modules/lag.py @@ -5,71 +5,74 @@ from . import _Module from .. import data from ..utils import log -from ..augment import devices - -# -# Checks if 2 lists have the same elements, independent of order -# -def same_list(l1:BoxList,l2:BoxList) -> bool: - for l in l1: - if l not in l2: - return False - return len(l1)==len(l2) +from ..augment import devices, links class LAG(_Module): """ - link_pre_transform: Create virtual LAG links + link_pre_transform: Process LAG links and add member links to the topology """ def link_pre_transform(self, link: Box, topology: Box) -> None: - if log.debug_active('vlan'): + if log.debug_active('lag'): print(f'LAG link_pre_transform for {link}') - if 'lag' in link and link.get('type',"")!="lag": - - # Lookup virtual LAG link with same id between same pair of nodes, create if not existing - vlag = None - for l in topology.links: - if 'lag' in l and l.get('type',"")=="lag" and l.lag.id == link.lag.id \ - and same_list(l.interfaces,link.interfaces): - vlag = l - break - - if vlag is None: - vlag = data.get_box(link) - vlag.type = "lag" - vlag.linkindex = len(topology.links) + 1 - vlag._linkname = f"links[{vlag.linkindex}]" - vlag.interfaces = [ i for i in link.interfaces ] # Make a deep copy, could use a set? - vlag.pop('mtu',None) # Remove any MTU attribute - topology.links.append(vlag) - if log.debug_active('vlan'): - print(f'LAG link_pre_transform created virtual link: {vlag}') + # Iterate over links with lag.id, skip over member links we added below + if 'lag' in link and link.get('type',"")!="p2p": + if 'members' not in link.lag: + log.error( + f'Link {link._linkname} defines "lag.id"={link.lag.id} but no "lag.members"', + category=log.MissingValue, + module='lag', + hint='lag') - # remove any VLAN attributes from original link - link.pop('vlan',None) + if len(link.interfaces)!=2: # Future: MC-LAG would be 3 + log.error( + 'Current LAG module only supports lags between exactly 2 nodes', + category=log.IncorrectAttr, + module='lag', + hint='lag') - """ - node_post_transform: Check for correct supported configuration of LAG - """ - def node_post_transform(self, node: Box, topology: Box) -> None: - features = devices.get_device_features(node,topology.defaults) - for i in node.interfaces: - # 1. Check if the interface is part of a LAG - if 'lag' in i: + # 1. Check that the nodes involved all support LAG + for i in link.interfaces: + n = topology.nodes[i.node] + features = devices.get_device_features(n,topology.defaults) if 'lag' not in features: log.error( - f'Node {node.name} does not support LAG configured on {i.ifname}', + f'Node {n.name} does not support LAG configured on link {link._linkname}', category=log.IncorrectAttr, module='lag', hint='lag') - _type = i.get("type") - if _type=="lag": - continue - elif _type!="p2p": + if isinstance(link.lag.members,int): + count = link.lag.members + link.lag.members = [] + for i in range(1,count): + link.lag.members.append( { 'interfaces': link.interfaces + [] } ) # Deep copy + + # 2. Normalize member links list + link.lag.members = links.adjust_link_list(link.lag.members,topology.nodes,f'lag{link.lag.id}.link[{{link_cnt}}]') + + if log.debug_active('lag'): + print(f'LAG link_pre_transform after normalizing members: {link}') + + # 3. Check that the nodes in member links match the ones declared for the LAG + declared = { l.node for l in link.interfaces } + for m in link.lag.members: + if any({ l.node not in declared for l in m.interfaces }): log.error( - f'Node {node.name} has a LAG configured on {i.ifname} which is not a p2p link ({_type})', + f'Nodes {m.interfaces} in member link {m._linkname} do not match with LAG {link.lag.id}: {declared}', category=log.IncorrectAttr, module='lag', hint='lag') + + # Add lag ID and append + m.lag.id = link.lag.id + m.linkindex = len(topology.links)+1 + m.type = 'p2p' + m.prefix = False # Disable IP assignment + if 'mtu' in link: + m.mtu = link.mtu # Copy any MTU setting + topology.links.append( m ) + + link.type = 'lag' + # Link code marks it as a 'virtual_interface' diff --git a/netsim/modules/lag.yml b/netsim/modules/lag.yml index 1542e86cbc..530067e262 100644 --- a/netsim/modules/lag.yml +++ b/netsim/modules/lag.yml @@ -14,5 +14,6 @@ attributes: id: { type: int, _required: True } lacp: lacp_mode: + members: interface: lacp_mode: diff --git a/netsim/providers/clab.yml b/netsim/providers/clab.yml index 503ed83750..ef91201fb2 100644 --- a/netsim/providers/clab.yml +++ b/netsim/providers/clab.yml @@ -20,6 +20,7 @@ cleanup: [ clab.yml, clab_files ] bridge_type: bridge # Use 'ovs-bridge' to create Openvswitch bridges runtime: docker # Default runtime, see Containerlab documentation kmods: + lag: [ bonding ] mpls: [ mpls-router, mpls-iptunnel ] sr: [ mpls-router, mpls-iptunnel ] vxlan: [ vxlan, udp_tunnel, ip6_udp_tunnel ] diff --git a/tests/integration/lag/01-l3-lag.yml b/tests/integration/lag/01-l3-lag.yml index e877482273..7f06856921 100644 --- a/tests/integration/lag/01-l3-lag.yml +++ b/tests/integration/lag/01-l3-lag.yml @@ -10,15 +10,13 @@ groups: provider: clab switches: members: [ r1,r2 ] - module: [lag,ospf] + module: [ lag, ospf ] links: -- r1: +- r1: # Need valid nodes on the link r2: lag.id: 1 -- r2: - r1: - lag.id: 1 + lag.members: 2 - h1-r1 - r2-h2 diff --git a/tests/integration/lag/01-l3-lag-with-vlans.yml b/tests/integration/lag/02-l3-lag-with-vlans.yml similarity index 87% rename from tests/integration/lag/01-l3-lag-with-vlans.yml rename to tests/integration/lag/02-l3-lag-with-vlans.yml index 428b080ba3..3563750e43 100644 --- a/tests/integration/lag/01-l3-lag-with-vlans.yml +++ b/tests/integration/lag/02-l3-lag-with-vlans.yml @@ -20,10 +20,10 @@ links: - r1: r2: lag.id: 1 - vlan.trunk: [ v1, v2 ] # Note that any trunk MUST be declared on the first link in the LAG -- r1: - r2: - lag.id: 1 + vlan.trunk: [ v1, v2 ] + lag.members: + - r1-r2 + - r1-r2 - r1: vlan.access: v1 h1: diff --git a/tests/topology/expected/lag-l3-vlan-trunk.yml b/tests/topology/expected/lag-l3-vlan-trunk.yml index a65dbef94e..23bf858cad 100644 --- a/tests/topology/expected/lag-l3-vlan-trunk.yml +++ b/tests/topology/expected/lag-l3-vlan-trunk.yml @@ -5,6 +5,72 @@ lag: lacp: fast lacp_mode: active links: +- bridge: input_1 + interfaces: + - ifindex: 30000 + ifname: bond0 + node: r1 + vlan: + trunk: + v1: {} + v2: {} + - ifindex: 30000 + ifname: bond0 + node: r2 + vlan: + trunk: + v1: {} + v2: {} + lag: + id: 1 + members: + - _linkname: lag1.link[1] + interfaces: + - ifindex: 1 + ifname: eth1 + node: r1 + - ifindex: 1 + ifname: eth1 + node: r2 + lag: + id: 1 + linkindex: 2 + prefix: false + type: p2p + - _linkname: lag1.link[2] + interfaces: + - ifindex: 2 + ifname: eth2 + node: r1 + - ifindex: 2 + ifname: eth2 + node: r2 + lag: + id: 1 + linkindex: 3 + prefix: false + type: p2p + - _linkname: lag1.link[3] + interfaces: + - ifindex: 3 + ifname: eth3 + node: r1 + - ifindex: 3 + ifname: eth3 + node: r2 + lag: + id: 1 + linkindex: 4 + prefix: false + type: p2p + linkindex: 1 + node_count: 2 + prefix: {} + type: lag + vlan: + trunk: + v1: {} + v2: {} - interfaces: - ifindex: 1 ifname: eth1 @@ -14,8 +80,9 @@ links: node: r2 lag: id: 1 - linkindex: 1 + linkindex: 2 node_count: 2 + prefix: false type: p2p - interfaces: - ifindex: 2 @@ -26,35 +93,23 @@ links: node: r2 lag: id: 1 - linkindex: 2 + linkindex: 3 node_count: 2 + prefix: false type: p2p -- bridge: input_3 - interfaces: - - ifindex: 30000 - ifname: bond0 +- interfaces: + - ifindex: 3 + ifname: eth3 node: r1 - vlan: - trunk: - v1: {} - v2: {} - - ifindex: 30000 - ifname: bond0 + - ifindex: 3 + ifname: eth3 node: r2 - vlan: - trunk: - v1: {} - v2: {} lag: id: 1 - linkindex: 3 + linkindex: 4 node_count: 2 - prefix: {} - type: lag - vlan: - trunk: - v1: {} - v2: {} + prefix: false + type: p2p module: - lag - vlan @@ -76,11 +131,106 @@ nodes: hostname: clab-input-r1 id: 1 interfaces: + - ifindex: 30000 + ifname: bond0 + lag: + id: 1 + members: + - _linkname: lag1.link[1] + interfaces: + - ifindex: 1 + ifname: eth1 + node: r1 + - ifindex: 1 + ifname: eth1 + node: r2 + lag: + id: 1 + linkindex: 2 + prefix: false + type: p2p + - _linkname: lag1.link[2] + interfaces: + - ifindex: 2 + ifname: eth2 + node: r1 + - ifindex: 2 + ifname: eth2 + node: r2 + lag: + id: 1 + linkindex: 3 + prefix: false + type: p2p + - _linkname: lag1.link[3] + interfaces: + - ifindex: 3 + ifname: eth3 + node: r1 + - ifindex: 3 + ifname: eth3 + node: r2 + lag: + id: 1 + linkindex: 4 + prefix: false + type: p2p + linkindex: 1 + mtu: 1500 + name: r1 -> r2 + neighbors: + - ifname: bond0 + lag: + id: 1 + members: + - _linkname: lag1.link[1] + interfaces: + - ifindex: 1 + ifname: eth1 + node: r1 + - ifindex: 1 + ifname: eth1 + node: r2 + lag: + id: 1 + linkindex: 2 + prefix: false + type: p2p + - _linkname: lag1.link[2] + interfaces: + - ifindex: 2 + ifname: eth2 + node: r1 + - ifindex: 2 + ifname: eth2 + node: r2 + lag: + id: 1 + linkindex: 3 + prefix: false + type: p2p + - _linkname: lag1.link[3] + interfaces: + - ifindex: 3 + ifname: eth3 + node: r1 + - ifindex: 3 + ifname: eth3 + node: r2 + lag: + id: 1 + linkindex: 4 + prefix: false + type: p2p + node: r2 + subif_index: 2 + type: lag + virtual_interface: true - ifindex: 1 ifname: eth1 lag: id: 1 - linkindex: 1 + linkindex: 2 mtu: 1500 name: r1 -> r2 neighbors: @@ -93,7 +243,7 @@ nodes: ifname: eth2 lag: id: 1 - linkindex: 2 + linkindex: 3 mtu: 1500 name: r1 -> r2 neighbors: @@ -102,21 +252,20 @@ nodes: id: 1 node: r2 type: p2p - - ifindex: 30000 - ifname: bond0 + - ifindex: 3 + ifname: eth3 lag: id: 1 - linkindex: 3 + linkindex: 4 + mtu: 1500 name: r1 -> r2 neighbors: - - ifname: bond0 + - ifname: eth3 lag: id: 1 node: r2 - subif_index: 2 - type: lag - virtual_interface: true - - ifindex: 3 + type: p2p + - ifindex: 4 ifname: bond0.1000 parent_ifindex: 30000 parent_ifname: bond0 @@ -125,7 +274,7 @@ nodes: vlan: access: v1 access_id: 1000 - - ifindex: 4 + - ifindex: 5 ifname: bond0.1001 parent_ifindex: 30000 parent_ifname: bond0 @@ -135,7 +284,7 @@ nodes: access: v2 access_id: 1001 - bridge_group: 1 - ifindex: 5 + ifindex: 6 ifname: vlan1000 ipv4: 172.16.0.1/24 name: VLAN v1 (1000) -> [r2] @@ -148,7 +297,7 @@ nodes: vlan: mode: irb - bridge_group: 2 - ifindex: 6 + ifindex: 7 ifname: vlan1001 ipv4: 172.16.1.1/24 name: VLAN v2 (1001) -> [r2] @@ -212,11 +361,106 @@ nodes: hostname: clab-input-r2 id: 2 interfaces: + - ifindex: 30000 + ifname: bond0 + lag: + id: 1 + members: + - _linkname: lag1.link[1] + interfaces: + - ifindex: 1 + ifname: eth1 + node: r1 + - ifindex: 1 + ifname: eth1 + node: r2 + lag: + id: 1 + linkindex: 2 + prefix: false + type: p2p + - _linkname: lag1.link[2] + interfaces: + - ifindex: 2 + ifname: eth2 + node: r1 + - ifindex: 2 + ifname: eth2 + node: r2 + lag: + id: 1 + linkindex: 3 + prefix: false + type: p2p + - _linkname: lag1.link[3] + interfaces: + - ifindex: 3 + ifname: eth3 + node: r1 + - ifindex: 3 + ifname: eth3 + node: r2 + lag: + id: 1 + linkindex: 4 + prefix: false + type: p2p + linkindex: 1 + mtu: 1500 + name: r2 -> r1 + neighbors: + - ifname: bond0 + lag: + id: 1 + members: + - _linkname: lag1.link[1] + interfaces: + - ifindex: 1 + ifname: eth1 + node: r1 + - ifindex: 1 + ifname: eth1 + node: r2 + lag: + id: 1 + linkindex: 2 + prefix: false + type: p2p + - _linkname: lag1.link[2] + interfaces: + - ifindex: 2 + ifname: eth2 + node: r1 + - ifindex: 2 + ifname: eth2 + node: r2 + lag: + id: 1 + linkindex: 3 + prefix: false + type: p2p + - _linkname: lag1.link[3] + interfaces: + - ifindex: 3 + ifname: eth3 + node: r1 + - ifindex: 3 + ifname: eth3 + node: r2 + lag: + id: 1 + linkindex: 4 + prefix: false + type: p2p + node: r1 + subif_index: 2 + type: lag + virtual_interface: true - ifindex: 1 ifname: eth1 lag: id: 1 - linkindex: 1 + linkindex: 2 mtu: 1500 name: r2 -> r1 neighbors: @@ -229,7 +473,7 @@ nodes: ifname: eth2 lag: id: 1 - linkindex: 2 + linkindex: 3 mtu: 1500 name: r2 -> r1 neighbors: @@ -238,21 +482,20 @@ nodes: id: 1 node: r1 type: p2p - - ifindex: 30000 - ifname: bond0 + - ifindex: 3 + ifname: eth3 lag: id: 1 - linkindex: 3 + linkindex: 4 + mtu: 1500 name: r2 -> r1 neighbors: - - ifname: bond0 + - ifname: eth3 lag: id: 1 node: r1 - subif_index: 2 - type: lag - virtual_interface: true - - ifindex: 3 + type: p2p + - ifindex: 4 ifname: bond0.1000 parent_ifindex: 30000 parent_ifname: bond0 @@ -261,7 +504,7 @@ nodes: vlan: access: v1 access_id: 1000 - - ifindex: 4 + - ifindex: 5 ifname: bond0.1001 parent_ifindex: 30000 parent_ifname: bond0 @@ -271,7 +514,7 @@ nodes: access: v2 access_id: 1001 - bridge_group: 1 - ifindex: 5 + ifindex: 6 ifname: vlan1000 ipv4: 172.16.0.2/24 name: VLAN v1 (1000) -> [r1] @@ -284,7 +527,7 @@ nodes: vlan: mode: irb - bridge_group: 2 - ifindex: 6 + ifindex: 7 ifname: vlan1001 ipv4: 172.16.1.2/24 name: VLAN v2 (1001) -> [r1] diff --git a/tests/topology/expected/lag-l3.yml b/tests/topology/expected/lag-l3.yml index 809219ec64..e0edb93fbb 100644 --- a/tests/topology/expected/lag-l3.yml +++ b/tests/topology/expected/lag-l3.yml @@ -5,32 +5,7 @@ lag: lacp: fast lacp_mode: active links: -- interfaces: - - ifindex: 1 - ifname: eth1 - node: r1 - - ifindex: 1 - ifname: eth1 - node: r2 - lag: - id: 1 - linkindex: 1 - mtu: 1600 - node_count: 2 - type: p2p -- interfaces: - - ifindex: 2 - ifname: eth2 - node: r2 - - ifindex: 2 - ifname: eth2 - node: r1 - lag: - id: 1 - linkindex: 2 - node_count: 2 - type: p2p -- bridge: input_3 +- bridge: input_1 interfaces: - ifindex: 30000 ifname: bond0 @@ -42,11 +17,41 @@ links: node: r2 lag: id: 1 - linkindex: 3 + members: + - _linkname: lag1.link[1] + interfaces: + - ifindex: 1 + ifname: eth1 + node: r1 + - ifindex: 1 + ifname: eth1 + node: r2 + lag: + id: 1 + linkindex: 2 + mtu: 1600 + prefix: false + type: p2p + linkindex: 1 + mtu: 1600 node_count: 2 prefix: ipv4: 10.1.0.0/30 type: lag +- interfaces: + - ifindex: 1 + ifname: eth1 + node: r1 + - ifindex: 1 + ifname: eth1 + node: r2 + lag: + id: 1 + linkindex: 2 + mtu: 1600 + node_count: 2 + prefix: false + type: p2p module: - lag name: input @@ -67,47 +72,65 @@ nodes: hostname: clab-input-r1 id: 1 interfaces: - - ifindex: 1 - ifname: eth1 + - ifindex: 30000 + ifname: bond0 + ipv4: 10.1.0.1/30 lag: id: 1 + members: + - _linkname: lag1.link[1] + interfaces: + - ifindex: 1 + ifname: eth1 + node: r1 + - ifindex: 1 + ifname: eth1 + node: r2 + lag: + id: 1 + linkindex: 2 + mtu: 1600 + prefix: false + type: p2p linkindex: 1 mtu: 1600 name: r1 -> r2 neighbors: - - ifname: eth1 + - ifname: bond0 + ipv4: 10.1.0.2/30 lag: id: 1 + members: + - _linkname: lag1.link[1] + interfaces: + - ifindex: 1 + ifname: eth1 + node: r1 + - ifindex: 1 + ifname: eth1 + node: r2 + lag: + id: 1 + linkindex: 2 + mtu: 1600 + prefix: false + type: p2p node: r2 - type: p2p - - ifindex: 2 - ifname: eth2 + type: lag + virtual_interface: true + - ifindex: 1 + ifname: eth1 lag: id: 1 linkindex: 2 - mtu: 1500 + mtu: 1600 name: r1 -> r2 neighbors: - - ifname: eth2 + - ifname: eth1 lag: id: 1 node: r2 type: p2p - - ifindex: 30000 - ifname: bond0 - ipv4: 10.1.0.1/30 - lag: - id: 1 - linkindex: 3 - name: r1 -> r2 - neighbors: - - ifname: bond0 - ipv4: 10.1.0.2/30 - lag: - id: 1 - node: r2 - type: lag - virtual_interface: true lag: lacp: fast lacp_mode: active @@ -142,47 +165,65 @@ nodes: hostname: clab-input-r2 id: 2 interfaces: - - ifindex: 1 - ifname: eth1 + - ifindex: 30000 + ifname: bond0 + ipv4: 10.1.0.2/30 lag: id: 1 + members: + - _linkname: lag1.link[1] + interfaces: + - ifindex: 1 + ifname: eth1 + node: r1 + - ifindex: 1 + ifname: eth1 + node: r2 + lag: + id: 1 + linkindex: 2 + mtu: 1600 + prefix: false + type: p2p linkindex: 1 mtu: 1600 name: r2 -> r1 neighbors: - - ifname: eth1 + - ifname: bond0 + ipv4: 10.1.0.1/30 lag: id: 1 + members: + - _linkname: lag1.link[1] + interfaces: + - ifindex: 1 + ifname: eth1 + node: r1 + - ifindex: 1 + ifname: eth1 + node: r2 + lag: + id: 1 + linkindex: 2 + mtu: 1600 + prefix: false + type: p2p node: r1 - type: p2p - - ifindex: 2 - ifname: eth2 + type: lag + virtual_interface: true + - ifindex: 1 + ifname: eth1 lag: id: 1 linkindex: 2 - mtu: 1500 + mtu: 1600 name: r2 -> r1 neighbors: - - ifname: eth2 + - ifname: eth1 lag: id: 1 node: r1 type: p2p - - ifindex: 30000 - ifname: bond0 - ipv4: 10.1.0.2/30 - lag: - id: 1 - linkindex: 3 - name: r2 -> r1 - neighbors: - - ifname: bond0 - ipv4: 10.1.0.1/30 - lag: - id: 1 - node: r1 - type: lag - virtual_interface: true lag: lacp: fast lacp_mode: active diff --git a/tests/topology/input/lag-l3-vlan-trunk.yml b/tests/topology/input/lag-l3-vlan-trunk.yml index 7cf347c605..57468e2c76 100644 --- a/tests/topology/input/lag-l3-vlan-trunk.yml +++ b/tests/topology/input/lag-l3-vlan-trunk.yml @@ -1,6 +1,5 @@ # -# Basic L3 LAG with VLANs example - ensure virtual 'lag' interfaces get created with L3 attributes moved from first physical link, -# and VLAN interfaces are associated with the correct elements +# Basic L3 LAG with VLANs example - 3 member links spelled out # defaults: @@ -17,8 +16,7 @@ nodes: [ r1, r2 ] links: - r1: r2: - lag.id: 1 vlan.trunk: [v1,v2] -- r1: - r2: - lag.id: 1 + lag: + id : 1 + members: [ r1-r2, r1-r2, r1-r2 ] diff --git a/tests/topology/input/lag-l3.yml b/tests/topology/input/lag-l3.yml index fd5643ad97..92b24649fe 100644 --- a/tests/topology/input/lag-l3.yml +++ b/tests/topology/input/lag-l3.yml @@ -1,5 +1,5 @@ # -# Basic L3 LAG example - ensure virtual 'lag' interfaces get created with L3 attributes moved from first physical link +# Basic L3 LAG example - single lag with 2 member links and custom MTU setting # defaults: @@ -12,7 +12,5 @@ links: - r1: r2: lag.id: 1 - mtu: 1600 # Test that mtu attribute is not applied to LAG interface -- r2: # Test that the order of declaration of nodes doesn't affect the outcome - r1: - lag.id: 1 + lag.members: 2 + mtu: 1600 # Test that MTU is copied to member links From 130dc993c3b69a1744eaa5f7a688981b3cf74036 Mon Sep 17 00:00:00 2001 From: Jeroen van Bemmel Date: Fri, 4 Oct 2024 22:47:48 -0500 Subject: [PATCH 05/21] Check for passive lag_mode too --- netsim/modules/lag.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/netsim/modules/lag.py b/netsim/modules/lag.py index 8f1a7f6280..a219d759bc 100644 --- a/netsim/modules/lag.py +++ b/netsim/modules/lag.py @@ -42,6 +42,14 @@ def link_pre_transform(self, link: Box, topology: Box) -> None: category=log.IncorrectAttr, module='lag', hint='lag') + ATT = 'lag.lacp_mode' + lacp_mode = i.get(ATT) or link.get(ATT) or n.get(ATT) or topology.defaults.get(ATT) + if lacp_mode=='passive' and not features.lag.get('passive',False): + log.error( + f'Node {n.name} does not support passive LACP configured on link {link._linkname}', + category=log.IncorrectAttr, + module='lag', + hint='lag') if isinstance(link.lag.members,int): count = link.lag.members From 6fcf0b77558a0bcfba238e85c0204960e0c23be1 Mon Sep 17 00:00:00 2001 From: Jeroen van Bemmel Date: Wed, 9 Oct 2024 16:24:00 -0500 Subject: [PATCH 06/21] Minor edits --- netsim/modules/lag.py | 15 +++++++++------ netsim/modules/lag.yml | 2 +- tests/integration/lag/01-l3-lag.yml | 8 +++++--- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/netsim/modules/lag.py b/netsim/modules/lag.py index a219d759bc..a9a0ad925e 100644 --- a/netsim/modules/lag.py +++ b/netsim/modules/lag.py @@ -20,7 +20,7 @@ def link_pre_transform(self, link: Box, topology: Box) -> None: if 'lag' in link and link.get('type',"")!="p2p": if 'members' not in link.lag: log.error( - f'Link {link._linkname} defines "lag.id"={link.lag.id} but no "lag.members"', + f'Link {link._linkname} uses a lag but does not include "lag.members"', category=log.MissingValue, module='lag', hint='lag') @@ -54,11 +54,11 @@ def link_pre_transform(self, link: Box, topology: Box) -> None: if isinstance(link.lag.members,int): count = link.lag.members link.lag.members = [] - for i in range(1,count): + for i in range(0,count): link.lag.members.append( { 'interfaces': link.interfaces + [] } ) # Deep copy - # 2. Normalize member links list - link.lag.members = links.adjust_link_list(link.lag.members,topology.nodes,f'lag{link.lag.id}.link[{{link_cnt}}]') + # 2. Normalize member links list, using linkindex as globally unique lag ID + link.lag.members = links.adjust_link_list(link.lag.members,topology.nodes,f'lag{link.linkindex}.link[{{link_cnt}}]') if log.debug_active('lag'): print(f'LAG link_pre_transform after normalizing members: {link}') @@ -68,13 +68,13 @@ def link_pre_transform(self, link: Box, topology: Box) -> None: for m in link.lag.members: if any({ l.node not in declared for l in m.interfaces }): log.error( - f'Nodes {m.interfaces} in member link {m._linkname} do not match with LAG {link.lag.id}: {declared}', + f'Nodes {m.interfaces} in member link {m._linkname} do not match with LAG : {declared}', category=log.IncorrectAttr, module='lag', hint='lag') # Add lag ID and append - m.lag.id = link.lag.id + m.lag._parent_linkindex = link.linkindex m.linkindex = len(topology.links)+1 m.type = 'p2p' m.prefix = False # Disable IP assignment @@ -84,3 +84,6 @@ def link_pre_transform(self, link: Box, topology: Box) -> None: link.type = 'lag' # Link code marks it as a 'virtual_interface' + + if log.debug_active('lag'): + print(f'LAG after link_pre_transform: {link}') diff --git a/netsim/modules/lag.yml b/netsim/modules/lag.yml index 530067e262..f4ae36bc33 100644 --- a/netsim/modules/lag.yml +++ b/netsim/modules/lag.yml @@ -11,9 +11,9 @@ attributes: lacp: lacp_mode: link: # Most should be consistent across both interfaces on the link - id: { type: int, _required: True } lacp: lacp_mode: members: + ifindex: int # Optional, to control naming of the bonding interface interface: lacp_mode: diff --git a/tests/integration/lag/01-l3-lag.yml b/tests/integration/lag/01-l3-lag.yml index 7f06856921..7b6c366917 100644 --- a/tests/integration/lag/01-l3-lag.yml +++ b/tests/integration/lag/01-l3-lag.yml @@ -9,16 +9,18 @@ groups: device: linux provider: clab switches: - members: [ r1,r2 ] + members: [ r1,r2,r3 ] module: [ lag, ospf ] links: - r1: # Need valid nodes on the link r2: - lag.id: 1 + lag.members: 2 +- r2: # Need valid nodes on the link + r3: lag.members: 2 - h1-r1 -- r2-h2 +- r3-h2 validate: ping: From 01f84527f380c96dae0df0a5d2e8f22541cc72a7 Mon Sep 17 00:00:00 2001 From: Jeroen van Bemmel Date: Wed, 9 Oct 2024 18:23:56 -0500 Subject: [PATCH 07/21] Modified to be based on link groups --- netsim/ansible/templates/lag/cumulus.j2 | 4 +- netsim/ansible/templates/lag/frr.j2 | 24 ++++----- netsim/modules/lag.py | 71 +++++++++++-------------- netsim/modules/lag.yml | 6 ++- tests/integration/lag/01-l3-lag.yml | 20 ++++--- 5 files changed, 64 insertions(+), 61 deletions(-) diff --git a/netsim/ansible/templates/lag/cumulus.j2 b/netsim/ansible/templates/lag/cumulus.j2 index 43697e3e3c..2114a5a5cc 100644 --- a/netsim/ansible/templates/lag/cumulus.j2 +++ b/netsim/ansible/templates/lag/cumulus.j2 @@ -10,11 +10,11 @@ echo "LAG: creating bond interface(s)" auto {{ data.ifname }} iface {{ data.ifname }} pre-up ip link add {{ data.ifname }} type bond - bond-slaves {%- for i in interfaces if 'lag' in i and i.type=='p2p' and i.lag.id==data.lag.id %} {{ i.ifname }}{%- endfor %} + bond-slaves {%- for i in interfaces if 'parentindex' in i and i.parentindex==data.linkindex %} {{ i.ifname }}{%- endfor %} {% set _lacp = data.lag.lacp|default(lag.lacp) %} {% if _lacp=='slow' %} bond-lacp-rate slow -{% elif _lacp=='off' %} +{% elif _lacp=='off' or data.lag.mode|default(lag.mode)=="balance-xor" %} bond-mode balance-xor {% endif %} {% endmacro %} diff --git a/netsim/ansible/templates/lag/frr.j2 b/netsim/ansible/templates/lag/frr.j2 index cdfd8941ef..4559489d53 100644 --- a/netsim/ansible/templates/lag/frr.j2 +++ b/netsim/ansible/templates/lag/frr.j2 @@ -7,27 +7,25 @@ set -e # Exit immediately when any command fails # Create bonds for LAGs, if any. Requires kernel bonding module loaded # {% for l in interfaces if 'lag' in l %} -{% if l.type=='lag' %} +{% if 'parentindex' not in l %} {% set _lacp = l.lag.lacp|default(lag.lacp) %} {% set lacp_act = 'off' if _lacp=='off' else 'on' %} {% set lacp_rate = ('lacp_rate ' + _lacp) if _lacp!='off' else '' %} -{% set _p = "balance-xor" if _lacp=='off' else "802.3ad" %} -{% set mode = "mode " + _p + " xmit_hash_policy encap3+4" %} -ip link add dev {{l.ifname}} type bond {{mode}} lacp_active {{lacp_act}} {{lacp_rate}} || true -ip link set dev {{l.ifname}} down -{% else %} -ip link set dev {{ l.ifname }} down +{% set _m = l.mode|default(lag.mode) %} +{% if _m=="802.3ad" %} +{% set _m = _m + " xmit_hash_policy encap3+4" %} +{% endif %} +ip link add dev {{l.ifname}} type bond mode {{_m}} lacp_active {{lacp_act}} {{lacp_rate}} || true {% endif %} +ip link set dev {{ l.ifname }} down {% endfor %} {% for l in interfaces if 'lag' in l %} -{% if l.type=='lag' %} -ip link set dev {{l.ifname}} up -{% else %} -ip link set dev {{ l.ifname }} master {% for i in interfaces if i.type=='lag' and i.lag.id==l.lag.id %}{{ i.ifname }} -{% endfor %} -ip link set dev {{ l.ifname }} up +{% if 'parentindex' in l %} +ip link set dev {{ l.ifname }} master {% for i in interfaces if i.type=='lag' and i.linkindex==l.parentindex %}{{ i.ifname }} +{% endfor %} {% endif %} +ip link set dev {{ l.ifname }} up {% endfor %} exit 0 \ No newline at end of file diff --git a/netsim/modules/lag.py b/netsim/modules/lag.py index a9a0ad925e..97b300a313 100644 --- a/netsim/modules/lag.py +++ b/netsim/modules/lag.py @@ -1,5 +1,6 @@ import typing import netaddr +import re from box import Box, BoxList from . import _Module @@ -7,6 +8,8 @@ from ..utils import log from ..augment import devices, links +GROUPNAME = re.compile(r'^links\[(?P\w+)\]\[\d+\]') + class LAG(_Module): """ @@ -16,12 +19,12 @@ def link_pre_transform(self, link: Box, topology: Box) -> None: if log.debug_active('lag'): print(f'LAG link_pre_transform for {link}') - # Iterate over links with lag.id, skip over member links we added below - if 'lag' in link and link.get('type',"")!="p2p": - if 'members' not in link.lag: - log.error( - f'Link {link._linkname} uses a lag but does not include "lag.members"', - category=log.MissingValue, + # Iterate over links with type lag, created for link group(s) + if link.get('type',"")=="lag" and not ('lag' in link and '_parent' in link.lag): + if not GROUPNAME.match(link._linkname): + log.error( + f'LAG link {link._linkname} is not part of a link group', + category=log.IncorrectAttr, module='lag', hint='lag') @@ -51,39 +54,29 @@ def link_pre_transform(self, link: Box, topology: Box) -> None: module='lag', hint='lag') - if isinstance(link.lag.members,int): - count = link.lag.members - link.lag.members = [] - for i in range(0,count): - link.lag.members.append( { 'interfaces': link.interfaces + [] } ) # Deep copy - - # 2. Normalize member links list, using linkindex as globally unique lag ID - link.lag.members = links.adjust_link_list(link.lag.members,topology.nodes,f'lag{link.linkindex}.link[{{link_cnt}}]') - - if log.debug_active('lag'): - print(f'LAG link_pre_transform after normalizing members: {link}') - - # 3. Check that the nodes in member links match the ones declared for the LAG - declared = { l.node for l in link.interfaces } - for m in link.lag.members: - if any({ l.node not in declared for l in m.interfaces }): - log.error( - f'Nodes {m.interfaces} in member link {m._linkname} do not match with LAG : {declared}', - category=log.IncorrectAttr, - module='lag', - hint='lag') - - # Add lag ID and append - m.lag._parent_linkindex = link.linkindex - m.linkindex = len(topology.links)+1 - m.type = 'p2p' - m.prefix = False # Disable IP assignment - if 'mtu' in link: - m.mtu = link.mtu # Copy any MTU setting - topology.links.append( m ) + group_name = GROUPNAME.search(link._linkname).group("group") + + # Find parent virtual link, create if not existing + parent = [ l for l in topology.links if l.get("type")=="lag" and l._linkname == group_name ] + if not parent: + parent = data.get_box(link) + parent._linkname = group_name + parent.linkindex = len(topology.links) + 1 + parent.interfaces = link.interfaces + [] # Deep copy + parent.lag._parent = True # Set flag for filtering + topology.links.append( parent ) + if log.debug_active('lag'): + print(f'LAG link_pre_transform created virtual parent {parent}') + else: + parent = parent[0] + # parent.interfaces.extend( link.interfaces ) # results in duplicate bond interfaces? - link.type = 'lag' - # Link code marks it as a 'virtual_interface' + # Modify the LAG member link + link.type = "p2p" # Change type back to p2p + link.lag = link.lag or {} # ..and make sure template code can check for lag links + link.prefix = False # Disallow IP addressing + link.pop('vlan',None) # Remove any VLAN, moved to the virtual lag interface + link.parentindex = parent.linkindex # Link physical interface to its virtual parent if log.debug_active('lag'): - print(f'LAG after link_pre_transform: {link}') + print(f'After LAG link_pre_transform: {link}') diff --git a/netsim/modules/lag.yml b/netsim/modules/lag.yml index f4ae36bc33..6878ba1075 100644 --- a/netsim/modules/lag.yml +++ b/netsim/modules/lag.yml @@ -2,18 +2,22 @@ # lacp: "fast" # Link Aggregation Control Protocol, standby signalling through link down not working lacp_mode: "active" # At least 1 side must be active +mode: "802.3ad" # Default to active/active with LACP attributes: global: lacp: { type: str, valid_values: [ "off", "slow", "fast" ] } lacp_mode: { type: str, valid_values: [ "passive", "active" ] } + mode: { type: str, valid_values: [ "802.3ad", "balance-xor", "active-backup" ] } node: lacp: lacp_mode: + mode: { type: str, valid_values: [ "802.3ad", "balance-xor", "active-backup" ] } link: # Most should be consistent across both interfaces on the link lacp: lacp_mode: + lagindex: int # Optional, to control naming of the bonding interface members: - ifindex: int # Optional, to control naming of the bonding interface + mode: { type: str, valid_values: [ "802.3ad", "balance-xor", "active-backup" ] } interface: lacp_mode: diff --git a/tests/integration/lag/01-l3-lag.yml b/tests/integration/lag/01-l3-lag.yml index 7b6c366917..0259333f8b 100644 --- a/tests/integration/lag/01-l3-lag.yml +++ b/tests/integration/lag/01-l3-lag.yml @@ -13,12 +13,20 @@ groups: module: [ lag, ospf ] links: -- r1: # Need valid nodes on the link - r2: - lag.members: 2 -- r2: # Need valid nodes on the link - r3: - lag.members: 2 +- group: lag1_2 + type: lag + members: [r1-r2,r1-r2] +- group: lag2_3 + type: lag + members: + - r2: + ifindex: 3 + r3: + ifindex: 3 + - r2: + ifindex: 4 + r3: + ifindex: 4 - h1-r1 - r3-h2 From d4df18fc6b07b8c75fd6f8a98c97dbcc7afd7ae4 Mon Sep 17 00:00:00 2001 From: Jeroen van Bemmel Date: Wed, 9 Oct 2024 20:44:34 -0500 Subject: [PATCH 08/21] Add lag.ifindex --- docs/module/lag.md | 45 +++++++++---------- netsim/devices/cumulus.yml | 2 +- netsim/devices/frr.yml | 2 +- netsim/modules/lag.py | 22 +++++++-- netsim/modules/lag.yml | 5 ++- tests/integration/lag/01-l3-lag.yml | 1 + .../integration/lag/02-l3-lag-with-vlans.yml | 11 +++-- 7 files changed, 52 insertions(+), 36 deletions(-) diff --git a/docs/module/lag.md b/docs/module/lag.md index b185aca50b..51bd0c60e7 100644 --- a/docs/module/lag.md +++ b/docs/module/lag.md @@ -12,8 +12,9 @@ LAG is currently supported on these platforms: ## Parameters -The following parameters can be set globally or per node/link/interface: +The following parameters can be set globally or per node/link: +* **mode**: lag mode, one of "802.3ad" (default), "balance-xor" or "active-backup" * **lacp**: LACP protocol interval: "fast", "slow" or "off" Note that 'link down' is not easily detectable in a virtual environment with veth pairs, therefore it is strongly recommended @@ -21,11 +22,13 @@ The following parameters can be set globally or per node/link/interface: * **lacp_mode**: "active" or "passive" -By setting the **lag.id** parameter at the link level and defining **lag.members**, a *lag* type link is created with the given list or count of member links. +* **ifindex**: Optional parameter to control naming of the bonding device + +By setting the link type to **lag** for a link group, a *lag* type link is created with the given list of member links. ## Example -To create a LAG consisting of 2 links between 2 devices: +To create a LAG consisting of 2 links between devices 'r1' and 'r2': ``` module: [ lag ] @@ -33,28 +36,24 @@ module: [ lag ] nodes: [ r1, r2 ] links: -- r1: - r2: - lag.id: 1 - lag.members: 2 +- group: lag1 + type: lag + members: [ r1-r2, r1-r2 ] ``` -Additional parameters such as vlan trunks, OSPF cost, etc. can be applied to such *lag* type links. +Additional parameters such as vlan trunks, OSPF cost, etc. can be applied to such *lag* type link groups. -In case additional attributes are required for the member links, the following syntax can also be used +In case additional attributes are required for the member links, the members can be expanded: ``` links: -- r1: - r2: - lag: - id: 1 - members: - - r1: - ifindex: 49 # Use 100G links 1/1/49 and 1/1/50 - r2: - ifindex: 49 - - r1: - ifindex: 50 - r2: - ifindex: 50 +- group: lag1 + type: lag + members: + - r1: + ifindex: 49 # Use 100G links 1/1/49 and 1/1/50 + r2: + ifindex: 49 + - r1: + ifindex: 50 + r2: + ifindex: 50 ``` -Naturally, the links in lag.members can only use nodes associated with the lag link \ No newline at end of file diff --git a/netsim/devices/cumulus.yml b/netsim/devices/cumulus.yml index 66e0d942e6..f0fb938e55 100644 --- a/netsim/devices/cumulus.yml +++ b/netsim/devices/cumulus.yml @@ -3,7 +3,7 @@ description: Cumulus VX 4.x or 5.x configured without NVUE interface_name: swp{ifindex} loopback_interface_name: lo{ifindex if ifindex else ""} tunnel_interface_name: "tun{ifindex}" -lag_interface_name: "bond{ifindex}" +lag_interface_name: "bond{lag.ifindex}" mgmt_if: eth0 libvirt: image: CumulusCommunity/cumulus-vx:4.4.5 diff --git a/netsim/devices/frr.yml b/netsim/devices/frr.yml index c8c552e987..e7468eefda 100644 --- a/netsim/devices/frr.yml +++ b/netsim/devices/frr.yml @@ -4,7 +4,7 @@ interface_name: eth{ifindex} mgmt_if: eth0 loopback_interface_name: lo{ifindex if ifindex else ""} tunnel_interface_name: "tun{ifindex}" -lag_interface_name: "bond{ifindex}" +lag_interface_name: "bond{lag.ifindex}" routing: _rm_per_af: True group_vars: diff --git a/netsim/modules/lag.py b/netsim/modules/lag.py index 97b300a313..a1505dec52 100644 --- a/netsim/modules/lag.py +++ b/netsim/modules/lag.py @@ -3,15 +3,26 @@ import re from box import Box, BoxList -from . import _Module +from . import _Module, _dataplane from .. import data from ..utils import log from ..augment import devices, links -GROUPNAME = re.compile(r'^links\[(?P\w+)\]\[\d+\]') +GROUPNAME = re.compile(r'^links\[(?P[\w\-_]+)\]\[\d+\]') + +ID_SET = 'lag_id' + +def populate_lag_id_set(topology: Box) -> None: + _dataplane.create_id_set(ID_SET) + LAG_IDS = { l.lag.ifindex for l in topology.links if 'lag' in l and 'ifindex' in l.lag } + _dataplane.extend_id_set(ID_SET,LAG_IDS) + _dataplane.set_id_counter(ID_SET,topology.defaults.lag.start_lag_id,100) class LAG(_Module): + def module_pre_transform(self, topology: Box) -> None: + populate_lag_id_set(topology) + """ link_pre_transform: Process LAG links and add member links to the topology """ @@ -62,14 +73,17 @@ def link_pre_transform(self, link: Box, topology: Box) -> None: parent = data.get_box(link) parent._linkname = group_name parent.linkindex = len(topology.links) + 1 - parent.interfaces = link.interfaces + [] # Deep copy + parent.interfaces = link.interfaces + [] # Deep copy, assumes all links have same 2 nodes parent.lag._parent = True # Set flag for filtering + if 'ifindex' not in parent.lag: # Use given lag.ifindex, if any + parent.lag.ifindex = _dataplane.get_next_id(ID_SET) topology.links.append( parent ) if log.debug_active('lag'): print(f'LAG link_pre_transform created virtual parent {parent}') else: parent = parent[0] - # parent.interfaces.extend( link.interfaces ) # results in duplicate bond interfaces? + # For future mc-lag: add any new nodes + # parent.interfaces.extend( [ n for n in link.interfaces if n not in parent.interfaces ] ) # Modify the LAG member link link.type = "p2p" # Change type back to p2p diff --git a/netsim/modules/lag.yml b/netsim/modules/lag.yml index 6878ba1075..ca88794940 100644 --- a/netsim/modules/lag.yml +++ b/netsim/modules/lag.yml @@ -1,5 +1,8 @@ # LAG default settings and attributes # +no_propagate: [ start_lag_id ] +start_lag_id: 0 # Start naming bonding interfaces using this lag.ifindex + lacp: "fast" # Link Aggregation Control Protocol, standby signalling through link down not working lacp_mode: "active" # At least 1 side must be active mode: "802.3ad" # Default to active/active with LACP @@ -16,7 +19,7 @@ attributes: link: # Most should be consistent across both interfaces on the link lacp: lacp_mode: - lagindex: int # Optional, to control naming of the bonding interface + ifindex: int # Optional, to control naming of the bonding interface members: mode: { type: str, valid_values: [ "802.3ad", "balance-xor", "active-backup" ] } interface: diff --git a/tests/integration/lag/01-l3-lag.yml b/tests/integration/lag/01-l3-lag.yml index 0259333f8b..5edd236201 100644 --- a/tests/integration/lag/01-l3-lag.yml +++ b/tests/integration/lag/01-l3-lag.yml @@ -18,6 +18,7 @@ links: members: [r1-r2,r1-r2] - group: lag2_3 type: lag + lag.ifindex: 0 # Name it as 'bond0' -> First one becomes 'bond1' members: - r2: ifindex: 3 diff --git a/tests/integration/lag/02-l3-lag-with-vlans.yml b/tests/integration/lag/02-l3-lag-with-vlans.yml index 3563750e43..90cf2d4015 100644 --- a/tests/integration/lag/02-l3-lag-with-vlans.yml +++ b/tests/integration/lag/02-l3-lag-with-vlans.yml @@ -17,11 +17,10 @@ vlans: v2: links: -- r1: - r2: - lag.id: 1 +- group: lag1-with-vlan + type: lag vlan.trunk: [ v1, v2 ] - lag.members: + members: - r1-r2 - r1-r2 - r1: @@ -39,12 +38,12 @@ links: validate: ping12: - description: Pinging H2 from H1 on VLAN 1 + description: Pinging H2 from H1 on VLAN v1 nodes: [ h1 ] wait_msg: Waiting for STP to enable the ports wait: 45 plugin: ping('h2') ping34: - description: Pinging H4 from H3 on VLAN 2 + description: Pinging H4 from H3 on VLAN v2 nodes: [ h3 ] plugin: ping('h4') \ No newline at end of file From bdea3f9e30c0ebb03c3fd36cc366765eb30303f0 Mon Sep 17 00:00:00 2001 From: Jeroen van Bemmel Date: Wed, 9 Oct 2024 20:48:04 -0500 Subject: [PATCH 09/21] Update tests --- tests/topology/expected/lag-l3-vlan-trunk.yml | 388 +++++------------- tests/topology/expected/lag-l3.yml | 217 +++++----- tests/topology/input/lag-l3-vlan-trunk.yml | 10 +- tests/topology/input/lag-l3.yml | 7 +- 4 files changed, 195 insertions(+), 427 deletions(-) diff --git a/tests/topology/expected/lag-l3-vlan-trunk.yml b/tests/topology/expected/lag-l3-vlan-trunk.yml index 23bf858cad..3b94d211f1 100644 --- a/tests/topology/expected/lag-l3-vlan-trunk.yml +++ b/tests/topology/expected/lag-l3-vlan-trunk.yml @@ -4,73 +4,8 @@ input: lag: lacp: fast lacp_mode: active + mode: 802.3ad links: -- bridge: input_1 - interfaces: - - ifindex: 30000 - ifname: bond0 - node: r1 - vlan: - trunk: - v1: {} - v2: {} - - ifindex: 30000 - ifname: bond0 - node: r2 - vlan: - trunk: - v1: {} - v2: {} - lag: - id: 1 - members: - - _linkname: lag1.link[1] - interfaces: - - ifindex: 1 - ifname: eth1 - node: r1 - - ifindex: 1 - ifname: eth1 - node: r2 - lag: - id: 1 - linkindex: 2 - prefix: false - type: p2p - - _linkname: lag1.link[2] - interfaces: - - ifindex: 2 - ifname: eth2 - node: r1 - - ifindex: 2 - ifname: eth2 - node: r2 - lag: - id: 1 - linkindex: 3 - prefix: false - type: p2p - - _linkname: lag1.link[3] - interfaces: - - ifindex: 3 - ifname: eth3 - node: r1 - - ifindex: 3 - ifname: eth3 - node: r2 - lag: - id: 1 - linkindex: 4 - prefix: false - type: p2p - linkindex: 1 - node_count: 2 - prefix: {} - type: lag - vlan: - trunk: - v1: {} - v2: {} - interfaces: - ifindex: 1 ifname: eth1 @@ -78,10 +13,10 @@ links: - ifindex: 1 ifname: eth1 node: r2 - lag: - id: 1 - linkindex: 2 + lag: {} + linkindex: 1 node_count: 2 + parentindex: 4 prefix: false type: p2p - interfaces: @@ -91,10 +26,10 @@ links: - ifindex: 2 ifname: eth2 node: r2 - lag: - id: 1 - linkindex: 3 + lag: {} + linkindex: 2 node_count: 2 + parentindex: 4 prefix: false type: p2p - interfaces: @@ -104,12 +39,39 @@ links: - ifindex: 3 ifname: eth3 node: r2 - lag: - id: 1 - linkindex: 4 + lag: {} + linkindex: 3 node_count: 2 + parentindex: 4 prefix: false type: p2p +- bridge: input_4 + interfaces: + - ifindex: 30000 + ifname: bond0 + node: r1 + vlan: + trunk: + v1: {} + v2: {} + - ifindex: 30000 + ifname: bond0 + node: r2 + vlan: + trunk: + v1: {} + v2: {} + lag: + _parent: true + ifindex: 0 + linkindex: 4 + node_count: 2 + prefix: {} + type: lag + vlan: + trunk: + v1: {} + v2: {} module: - lag - vlan @@ -131,140 +93,59 @@ nodes: hostname: clab-input-r1 id: 1 interfaces: - - ifindex: 30000 - ifname: bond0 - lag: - id: 1 - members: - - _linkname: lag1.link[1] - interfaces: - - ifindex: 1 - ifname: eth1 - node: r1 - - ifindex: 1 - ifname: eth1 - node: r2 - lag: - id: 1 - linkindex: 2 - prefix: false - type: p2p - - _linkname: lag1.link[2] - interfaces: - - ifindex: 2 - ifname: eth2 - node: r1 - - ifindex: 2 - ifname: eth2 - node: r2 - lag: - id: 1 - linkindex: 3 - prefix: false - type: p2p - - _linkname: lag1.link[3] - interfaces: - - ifindex: 3 - ifname: eth3 - node: r1 - - ifindex: 3 - ifname: eth3 - node: r2 - lag: - id: 1 - linkindex: 4 - prefix: false - type: p2p - linkindex: 1 - mtu: 1500 - name: r1 -> r2 - neighbors: - - ifname: bond0 - lag: - id: 1 - members: - - _linkname: lag1.link[1] - interfaces: - - ifindex: 1 - ifname: eth1 - node: r1 - - ifindex: 1 - ifname: eth1 - node: r2 - lag: - id: 1 - linkindex: 2 - prefix: false - type: p2p - - _linkname: lag1.link[2] - interfaces: - - ifindex: 2 - ifname: eth2 - node: r1 - - ifindex: 2 - ifname: eth2 - node: r2 - lag: - id: 1 - linkindex: 3 - prefix: false - type: p2p - - _linkname: lag1.link[3] - interfaces: - - ifindex: 3 - ifname: eth3 - node: r1 - - ifindex: 3 - ifname: eth3 - node: r2 - lag: - id: 1 - linkindex: 4 - prefix: false - type: p2p - node: r2 - subif_index: 2 - type: lag - virtual_interface: true - ifindex: 1 ifname: eth1 - lag: - id: 1 - linkindex: 2 + lag: {} + linkindex: 1 mtu: 1500 name: r1 -> r2 neighbors: - ifname: eth1 - lag: - id: 1 + lag: {} node: r2 + parentindex: 4 type: p2p - ifindex: 2 ifname: eth2 - lag: - id: 1 - linkindex: 3 + lag: {} + linkindex: 2 mtu: 1500 name: r1 -> r2 neighbors: - ifname: eth2 - lag: - id: 1 + lag: {} node: r2 + parentindex: 4 type: p2p - ifindex: 3 ifname: eth3 + lag: {} + linkindex: 3 + mtu: 1500 + name: r1 -> r2 + neighbors: + - ifname: eth3 + lag: {} + node: r2 + parentindex: 4 + type: p2p + - ifindex: 30000 + ifname: bond0 lag: - id: 1 + _parent: true + ifindex: 0 linkindex: 4 mtu: 1500 name: r1 -> r2 neighbors: - - ifname: eth3 + - ifname: bond0 lag: - id: 1 + _parent: true + ifindex: 0 node: r2 - type: p2p + subif_index: 2 + type: lag + virtual_interface: true - ifindex: 4 ifname: bond0.1000 parent_ifindex: 30000 @@ -312,6 +193,7 @@ nodes: lag: lacp: fast lacp_mode: active + mode: 802.3ad loopback: ifindex: 0 ifname: lo @@ -361,140 +243,59 @@ nodes: hostname: clab-input-r2 id: 2 interfaces: - - ifindex: 30000 - ifname: bond0 - lag: - id: 1 - members: - - _linkname: lag1.link[1] - interfaces: - - ifindex: 1 - ifname: eth1 - node: r1 - - ifindex: 1 - ifname: eth1 - node: r2 - lag: - id: 1 - linkindex: 2 - prefix: false - type: p2p - - _linkname: lag1.link[2] - interfaces: - - ifindex: 2 - ifname: eth2 - node: r1 - - ifindex: 2 - ifname: eth2 - node: r2 - lag: - id: 1 - linkindex: 3 - prefix: false - type: p2p - - _linkname: lag1.link[3] - interfaces: - - ifindex: 3 - ifname: eth3 - node: r1 - - ifindex: 3 - ifname: eth3 - node: r2 - lag: - id: 1 - linkindex: 4 - prefix: false - type: p2p - linkindex: 1 - mtu: 1500 - name: r2 -> r1 - neighbors: - - ifname: bond0 - lag: - id: 1 - members: - - _linkname: lag1.link[1] - interfaces: - - ifindex: 1 - ifname: eth1 - node: r1 - - ifindex: 1 - ifname: eth1 - node: r2 - lag: - id: 1 - linkindex: 2 - prefix: false - type: p2p - - _linkname: lag1.link[2] - interfaces: - - ifindex: 2 - ifname: eth2 - node: r1 - - ifindex: 2 - ifname: eth2 - node: r2 - lag: - id: 1 - linkindex: 3 - prefix: false - type: p2p - - _linkname: lag1.link[3] - interfaces: - - ifindex: 3 - ifname: eth3 - node: r1 - - ifindex: 3 - ifname: eth3 - node: r2 - lag: - id: 1 - linkindex: 4 - prefix: false - type: p2p - node: r1 - subif_index: 2 - type: lag - virtual_interface: true - ifindex: 1 ifname: eth1 - lag: - id: 1 - linkindex: 2 + lag: {} + linkindex: 1 mtu: 1500 name: r2 -> r1 neighbors: - ifname: eth1 - lag: - id: 1 + lag: {} node: r1 + parentindex: 4 type: p2p - ifindex: 2 ifname: eth2 - lag: - id: 1 - linkindex: 3 + lag: {} + linkindex: 2 mtu: 1500 name: r2 -> r1 neighbors: - ifname: eth2 - lag: - id: 1 + lag: {} node: r1 + parentindex: 4 type: p2p - ifindex: 3 ifname: eth3 + lag: {} + linkindex: 3 + mtu: 1500 + name: r2 -> r1 + neighbors: + - ifname: eth3 + lag: {} + node: r1 + parentindex: 4 + type: p2p + - ifindex: 30000 + ifname: bond0 lag: - id: 1 + _parent: true + ifindex: 0 linkindex: 4 mtu: 1500 name: r2 -> r1 neighbors: - - ifname: eth3 + - ifname: bond0 lag: - id: 1 + _parent: true + ifindex: 0 node: r1 - type: p2p + subif_index: 2 + type: lag + virtual_interface: true - ifindex: 4 ifname: bond0.1000 parent_ifindex: 30000 @@ -542,6 +343,7 @@ nodes: lag: lacp: fast lacp_mode: active + mode: 802.3ad loopback: ifindex: 0 ifname: lo diff --git a/tests/topology/expected/lag-l3.yml b/tests/topology/expected/lag-l3.yml index e0edb93fbb..5a7dd2bb92 100644 --- a/tests/topology/expected/lag-l3.yml +++ b/tests/topology/expected/lag-l3.yml @@ -4,8 +4,37 @@ input: lag: lacp: fast lacp_mode: active + mode: 802.3ad links: -- bridge: input_1 +- interfaces: + - ifindex: 1 + ifname: eth1 + node: r1 + - ifindex: 1 + ifname: eth1 + node: r2 + lag: {} + linkindex: 1 + mtu: 1600 + node_count: 2 + parentindex: 3 + prefix: false + type: p2p +- interfaces: + - ifindex: 2 + ifname: eth2 + node: r1 + - ifindex: 2 + ifname: eth2 + node: r2 + lag: {} + linkindex: 2 + mtu: 1600 + node_count: 2 + parentindex: 3 + prefix: false + type: p2p +- bridge: input_3 interfaces: - ifindex: 30000 ifname: bond0 @@ -16,42 +45,14 @@ links: ipv4: 10.1.0.2/30 node: r2 lag: - id: 1 - members: - - _linkname: lag1.link[1] - interfaces: - - ifindex: 1 - ifname: eth1 - node: r1 - - ifindex: 1 - ifname: eth1 - node: r2 - lag: - id: 1 - linkindex: 2 - mtu: 1600 - prefix: false - type: p2p - linkindex: 1 + _parent: true + ifindex: 0 + linkindex: 3 mtu: 1600 node_count: 2 prefix: ipv4: 10.1.0.0/30 type: lag -- interfaces: - - ifindex: 1 - ifname: eth1 - node: r1 - - ifindex: 1 - ifname: eth1 - node: r2 - lag: - id: 1 - linkindex: 2 - mtu: 1600 - node_count: 2 - prefix: false - type: p2p module: - lag name: input @@ -72,68 +73,52 @@ nodes: hostname: clab-input-r1 id: 1 interfaces: + - ifindex: 1 + ifname: eth1 + lag: {} + linkindex: 1 + mtu: 1600 + name: r1 -> r2 + neighbors: + - ifname: eth1 + lag: {} + node: r2 + parentindex: 3 + type: p2p + - ifindex: 2 + ifname: eth2 + lag: {} + linkindex: 2 + mtu: 1600 + name: r1 -> r2 + neighbors: + - ifname: eth2 + lag: {} + node: r2 + parentindex: 3 + type: p2p - ifindex: 30000 ifname: bond0 ipv4: 10.1.0.1/30 lag: - id: 1 - members: - - _linkname: lag1.link[1] - interfaces: - - ifindex: 1 - ifname: eth1 - node: r1 - - ifindex: 1 - ifname: eth1 - node: r2 - lag: - id: 1 - linkindex: 2 - mtu: 1600 - prefix: false - type: p2p - linkindex: 1 + _parent: true + ifindex: 0 + linkindex: 3 mtu: 1600 name: r1 -> r2 neighbors: - ifname: bond0 ipv4: 10.1.0.2/30 lag: - id: 1 - members: - - _linkname: lag1.link[1] - interfaces: - - ifindex: 1 - ifname: eth1 - node: r1 - - ifindex: 1 - ifname: eth1 - node: r2 - lag: - id: 1 - linkindex: 2 - mtu: 1600 - prefix: false - type: p2p + _parent: true + ifindex: 0 node: r2 type: lag virtual_interface: true - - ifindex: 1 - ifname: eth1 - lag: - id: 1 - linkindex: 2 - mtu: 1600 - name: r1 -> r2 - neighbors: - - ifname: eth1 - lag: - id: 1 - node: r2 - type: p2p lag: lacp: fast lacp_mode: active + mode: 802.3ad loopback: ifindex: 0 ifname: lo @@ -165,68 +150,52 @@ nodes: hostname: clab-input-r2 id: 2 interfaces: + - ifindex: 1 + ifname: eth1 + lag: {} + linkindex: 1 + mtu: 1600 + name: r2 -> r1 + neighbors: + - ifname: eth1 + lag: {} + node: r1 + parentindex: 3 + type: p2p + - ifindex: 2 + ifname: eth2 + lag: {} + linkindex: 2 + mtu: 1600 + name: r2 -> r1 + neighbors: + - ifname: eth2 + lag: {} + node: r1 + parentindex: 3 + type: p2p - ifindex: 30000 ifname: bond0 ipv4: 10.1.0.2/30 lag: - id: 1 - members: - - _linkname: lag1.link[1] - interfaces: - - ifindex: 1 - ifname: eth1 - node: r1 - - ifindex: 1 - ifname: eth1 - node: r2 - lag: - id: 1 - linkindex: 2 - mtu: 1600 - prefix: false - type: p2p - linkindex: 1 + _parent: true + ifindex: 0 + linkindex: 3 mtu: 1600 name: r2 -> r1 neighbors: - ifname: bond0 ipv4: 10.1.0.1/30 lag: - id: 1 - members: - - _linkname: lag1.link[1] - interfaces: - - ifindex: 1 - ifname: eth1 - node: r1 - - ifindex: 1 - ifname: eth1 - node: r2 - lag: - id: 1 - linkindex: 2 - mtu: 1600 - prefix: false - type: p2p + _parent: true + ifindex: 0 node: r1 type: lag virtual_interface: true - - ifindex: 1 - ifname: eth1 - lag: - id: 1 - linkindex: 2 - mtu: 1600 - name: r2 -> r1 - neighbors: - - ifname: eth1 - lag: - id: 1 - node: r1 - type: p2p lag: lacp: fast lacp_mode: active + mode: 802.3ad loopback: ifindex: 0 ifname: lo diff --git a/tests/topology/input/lag-l3-vlan-trunk.yml b/tests/topology/input/lag-l3-vlan-trunk.yml index 57468e2c76..152369fcee 100644 --- a/tests/topology/input/lag-l3-vlan-trunk.yml +++ b/tests/topology/input/lag-l3-vlan-trunk.yml @@ -1,5 +1,5 @@ # -# Basic L3 LAG with VLANs example - 3 member links spelled out +# Basic L3 LAG with VLANs example - 3 member links # defaults: @@ -14,9 +14,7 @@ vlans: nodes: [ r1, r2 ] links: -- r1: - r2: +- group: lag_with_trunk + type: lag vlan.trunk: [v1,v2] - lag: - id : 1 - members: [ r1-r2, r1-r2, r1-r2 ] + members: [ r1-r2, r1-r2, r1-r2 ] diff --git a/tests/topology/input/lag-l3.yml b/tests/topology/input/lag-l3.yml index 92b24649fe..0e27f0b0c1 100644 --- a/tests/topology/input/lag-l3.yml +++ b/tests/topology/input/lag-l3.yml @@ -9,8 +9,7 @@ defaults: module: [ lag ] nodes: [ r1, r2 ] links: -- r1: - r2: - lag.id: 1 - lag.members: 2 +- group: lag1 + type: lag mtu: 1600 # Test that MTU is copied to member links + members: [ r1-r2, r1-r2 ] From 292797650df48b2375531383b4abaadfc177b04c Mon Sep 17 00:00:00 2001 From: Jeroen van Bemmel Date: Wed, 9 Oct 2024 20:57:21 -0500 Subject: [PATCH 10/21] Improve error messages --- netsim/ansible/templates/initial/cumulus.j2 | 1 - netsim/ansible/templates/vlan/cumulus.j2 | 2 +- netsim/modules/lag.py | 12 ++++++------ 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/netsim/ansible/templates/initial/cumulus.j2 b/netsim/ansible/templates/initial/cumulus.j2 index 5b8bac2489..4ef95cff99 100644 --- a/netsim/ansible/templates/initial/cumulus.j2 +++ b/netsim/ansible/templates/initial/cumulus.j2 @@ -99,7 +99,6 @@ iface {{ l.ifname }} inet6 static iface {{ l.ifname }} mtu {{ l.mtu }} {% endif %} - {% endfor %} CONFIG # diff --git a/netsim/ansible/templates/vlan/cumulus.j2 b/netsim/ansible/templates/vlan/cumulus.j2 index f14dcede6f..741fb33e24 100644 --- a/netsim/ansible/templates/vlan/cumulus.j2 +++ b/netsim/ansible/templates/vlan/cumulus.j2 @@ -21,7 +21,7 @@ iface bridge {% set vids = vlans.values() | map(attribute='id') | sort | map('string') %} bridge-vids {{ ",".join(vids) }} {% endif %} -{% for ifdata in interfaces if ifdata.vlan is defined and ifdata.type!="svi" %} +{% for ifdata in interfaces if ifdata.vlan is defined and (ifdata.virtual_interface is not defined or ifdata.type=="lag") %} bridge-ports {{ ifdata.ifname }} {% endfor %} CONFIG diff --git a/netsim/modules/lag.py b/netsim/modules/lag.py index a1505dec52..05f828d890 100644 --- a/netsim/modules/lag.py +++ b/netsim/modules/lag.py @@ -24,7 +24,7 @@ def module_pre_transform(self, topology: Box) -> None: populate_lag_id_set(topology) """ - link_pre_transform: Process LAG links and add member links to the topology + link_pre_transform: Process LAG link groups and add virtual parent links to the topology """ def link_pre_transform(self, link: Box, topology: Box) -> None: if log.debug_active('lag'): @@ -38,15 +38,17 @@ def link_pre_transform(self, link: Box, topology: Box) -> None: category=log.IncorrectAttr, module='lag', hint='lag') + group_name = GROUPNAME.search(link._linkname).group("group") - if len(link.interfaces)!=2: # Future: MC-LAG would be 3 + # Check that lag member links have exactly 2 nodes + if len(link.interfaces)!=2: log.error( - 'Current LAG module only supports lags between exactly 2 nodes', + f'Links in LAG group {group_name} must have exactly 2 nodes', category=log.IncorrectAttr, module='lag', hint='lag') - # 1. Check that the nodes involved all support LAG + # 1. Check that the 2 nodes involved all (both) support LAG for i in link.interfaces: n = topology.nodes[i.node] features = devices.get_device_features(n,topology.defaults) @@ -65,8 +67,6 @@ def link_pre_transform(self, link: Box, topology: Box) -> None: module='lag', hint='lag') - group_name = GROUPNAME.search(link._linkname).group("group") - # Find parent virtual link, create if not existing parent = [ l for l in topology.links if l.get("type")=="lag" and l._linkname == group_name ] if not parent: From a0a3e0d49c9ea941969ba16b03f5009c0374b7c3 Mon Sep 17 00:00:00 2001 From: Jeroen van Bemmel Date: Thu, 10 Oct 2024 07:43:18 -0500 Subject: [PATCH 11/21] Fix errors --- netsim/modules/lag.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/netsim/modules/lag.py b/netsim/modules/lag.py index 05f828d890..a1f701c2ed 100644 --- a/netsim/modules/lag.py +++ b/netsim/modules/lag.py @@ -32,13 +32,14 @@ def link_pre_transform(self, link: Box, topology: Box) -> None: # Iterate over links with type lag, created for link group(s) if link.get('type',"")=="lag" and not ('lag' in link and '_parent' in link.lag): - if not GROUPNAME.match(link._linkname): + match = GROUPNAME.search(link._linkname) + if not match: log.error( f'LAG link {link._linkname} is not part of a link group', category=log.IncorrectAttr, module='lag', hint='lag') - group_name = GROUPNAME.search(link._linkname).group("group") + group_name = match.group("group") # Check that lag member links have exactly 2 nodes if len(link.interfaces)!=2: @@ -68,8 +69,8 @@ def link_pre_transform(self, link: Box, topology: Box) -> None: hint='lag') # Find parent virtual link, create if not existing - parent = [ l for l in topology.links if l.get("type")=="lag" and l._linkname == group_name ] - if not parent: + parents = [ l for l in topology.links if l.get("type")=="lag" and l._linkname == group_name ] + if not parents: parent = data.get_box(link) parent._linkname = group_name parent.linkindex = len(topology.links) + 1 @@ -81,7 +82,7 @@ def link_pre_transform(self, link: Box, topology: Box) -> None: if log.debug_active('lag'): print(f'LAG link_pre_transform created virtual parent {parent}') else: - parent = parent[0] + parent = parents[0] # For future mc-lag: add any new nodes # parent.interfaces.extend( [ n for n in link.interfaces if n not in parent.interfaces ] ) From 99b2391fb0537e0d79a3192de906993419dde803 Mon Sep 17 00:00:00 2001 From: Jeroen van Bemmel Date: Thu, 10 Oct 2024 07:51:15 -0500 Subject: [PATCH 12/21] Satisfy pyTest checks --- netsim/modules/lag.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/netsim/modules/lag.py b/netsim/modules/lag.py index a1f701c2ed..c41ec09433 100644 --- a/netsim/modules/lag.py +++ b/netsim/modules/lag.py @@ -32,14 +32,16 @@ def link_pre_transform(self, link: Box, topology: Box) -> None: # Iterate over links with type lag, created for link group(s) if link.get('type',"")=="lag" and not ('lag' in link and '_parent' in link.lag): - match = GROUPNAME.search(link._linkname) - if not match: - log.error( - f'LAG link {link._linkname} is not part of a link group', - category=log.IncorrectAttr, - module='lag', - hint='lag') - group_name = match.group("group") + _match = GROUPNAME.search(link._linkname) + if not _match: + log.error( + f'LAG link {link._linkname} is not part of a link group', + category=log.IncorrectAttr, + module='lag', + hint='lag') + group_name = "?" + else: + group_name = _match.group("group") # Check that lag member links have exactly 2 nodes if len(link.interfaces)!=2: From 00a8ed96117f9b10916a3cc504b11ce137de9572 Mon Sep 17 00:00:00 2001 From: Jeroen van Bemmel Date: Thu, 10 Oct 2024 14:20:48 -0500 Subject: [PATCH 13/21] Set _link_group attribute to track relation between links and groups, instead of reverse engineering through _linkname --- netsim/augment/links.py | 1 + netsim/modules/lag.py | 19 +++---------------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/netsim/augment/links.py b/netsim/augment/links.py index 5857d71cac..87d911994b 100644 --- a/netsim/augment/links.py +++ b/netsim/augment/links.py @@ -1056,6 +1056,7 @@ def expand_groups(topology: Box) -> None: continue # Report error and skip otherwise copy_group_data = data.get_box(link) # We'll copy all group data into member links + copy_group_data._link_group = link.group # Keep track of link group that members belong to for key in ['group','members','_linkname']: # Apart from the link name and copy_group_data.pop(key,None) diff --git a/netsim/modules/lag.py b/netsim/modules/lag.py index c41ec09433..dce241a77e 100644 --- a/netsim/modules/lag.py +++ b/netsim/modules/lag.py @@ -1,6 +1,5 @@ import typing import netaddr -import re from box import Box, BoxList from . import _Module, _dataplane @@ -8,8 +7,6 @@ from ..utils import log from ..augment import devices, links -GROUPNAME = re.compile(r'^links\[(?P[\w\-_]+)\]\[\d+\]') - ID_SET = 'lag_id' def populate_lag_id_set(topology: Box) -> None: @@ -31,18 +28,8 @@ def link_pre_transform(self, link: Box, topology: Box) -> None: print(f'LAG link_pre_transform for {link}') # Iterate over links with type lag, created for link group(s) - if link.get('type',"")=="lag" and not ('lag' in link and '_parent' in link.lag): - _match = GROUPNAME.search(link._linkname) - if not _match: - log.error( - f'LAG link {link._linkname} is not part of a link group', - category=log.IncorrectAttr, - module='lag', - hint='lag') - group_name = "?" - else: - group_name = _match.group("group") - + if link.get('type',"")=="lag" and '_link_group' in link: + group_name = link._link_group # Check that lag member links have exactly 2 nodes if len(link.interfaces)!=2: log.error( @@ -75,9 +62,9 @@ def link_pre_transform(self, link: Box, topology: Box) -> None: if not parents: parent = data.get_box(link) parent._linkname = group_name + parent.pop('_link_group',None) # Don't include parent in group parent.linkindex = len(topology.links) + 1 parent.interfaces = link.interfaces + [] # Deep copy, assumes all links have same 2 nodes - parent.lag._parent = True # Set flag for filtering if 'ifindex' not in parent.lag: # Use given lag.ifindex, if any parent.lag.ifindex = _dataplane.get_next_id(ID_SET) topology.links.append( parent ) From 4b956f59d7a53a15ad0983159416b8f4da56c6bb Mon Sep 17 00:00:00 2001 From: Jeroen van Bemmel Date: Thu, 10 Oct 2024 14:29:47 -0500 Subject: [PATCH 14/21] Update test results Cleanup _link_group attribute --- netsim/augment/links.py | 1 + tests/topology/expected/lag-l3-vlan-trunk.yml | 5 ----- tests/topology/expected/lag-l3.yml | 5 ----- 3 files changed, 1 insertion(+), 10 deletions(-) diff --git a/netsim/augment/links.py b/netsim/augment/links.py index 87d911994b..c30386f549 100644 --- a/netsim/augment/links.py +++ b/netsim/augment/links.py @@ -1103,3 +1103,4 @@ def cleanup(topology: Box) -> None: for link in topology.links: link.pop('_linkname',None) + link.pop('_link_group',None) diff --git a/tests/topology/expected/lag-l3-vlan-trunk.yml b/tests/topology/expected/lag-l3-vlan-trunk.yml index 3b94d211f1..7e0147840d 100644 --- a/tests/topology/expected/lag-l3-vlan-trunk.yml +++ b/tests/topology/expected/lag-l3-vlan-trunk.yml @@ -62,7 +62,6 @@ links: v1: {} v2: {} lag: - _parent: true ifindex: 0 linkindex: 4 node_count: 2 @@ -132,7 +131,6 @@ nodes: - ifindex: 30000 ifname: bond0 lag: - _parent: true ifindex: 0 linkindex: 4 mtu: 1500 @@ -140,7 +138,6 @@ nodes: neighbors: - ifname: bond0 lag: - _parent: true ifindex: 0 node: r2 subif_index: 2 @@ -282,7 +279,6 @@ nodes: - ifindex: 30000 ifname: bond0 lag: - _parent: true ifindex: 0 linkindex: 4 mtu: 1500 @@ -290,7 +286,6 @@ nodes: neighbors: - ifname: bond0 lag: - _parent: true ifindex: 0 node: r1 subif_index: 2 diff --git a/tests/topology/expected/lag-l3.yml b/tests/topology/expected/lag-l3.yml index 5a7dd2bb92..9379dfbb9f 100644 --- a/tests/topology/expected/lag-l3.yml +++ b/tests/topology/expected/lag-l3.yml @@ -45,7 +45,6 @@ links: ipv4: 10.1.0.2/30 node: r2 lag: - _parent: true ifindex: 0 linkindex: 3 mtu: 1600 @@ -101,7 +100,6 @@ nodes: ifname: bond0 ipv4: 10.1.0.1/30 lag: - _parent: true ifindex: 0 linkindex: 3 mtu: 1600 @@ -110,7 +108,6 @@ nodes: - ifname: bond0 ipv4: 10.1.0.2/30 lag: - _parent: true ifindex: 0 node: r2 type: lag @@ -178,7 +175,6 @@ nodes: ifname: bond0 ipv4: 10.1.0.2/30 lag: - _parent: true ifindex: 0 linkindex: 3 mtu: 1600 @@ -187,7 +183,6 @@ nodes: - ifname: bond0 ipv4: 10.1.0.1/30 lag: - _parent: true ifindex: 0 node: r1 type: lag From ef27e84d69aaa646757adc6cb356d5a2069afe19 Mon Sep 17 00:00:00 2001 From: J vanBemmel Date: Fri, 11 Oct 2024 08:33:13 -0500 Subject: [PATCH 15/21] Fix lag.mode --- netsim/ansible/templates/lag/frr.j2 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/netsim/ansible/templates/lag/frr.j2 b/netsim/ansible/templates/lag/frr.j2 index 4559489d53..5c8bd87fbb 100644 --- a/netsim/ansible/templates/lag/frr.j2 +++ b/netsim/ansible/templates/lag/frr.j2 @@ -11,7 +11,7 @@ set -e # Exit immediately when any command fails {% set _lacp = l.lag.lacp|default(lag.lacp) %} {% set lacp_act = 'off' if _lacp=='off' else 'on' %} {% set lacp_rate = ('lacp_rate ' + _lacp) if _lacp!='off' else '' %} -{% set _m = l.mode|default(lag.mode) %} +{% set _m = l.lag.mode|default(lag.mode) %} {% if _m=="802.3ad" %} {% set _m = _m + " xmit_hash_policy encap3+4" %} {% endif %} @@ -28,4 +28,4 @@ ip link set dev {{ l.ifname }} master {% for i in interfaces if i.type=='lag' an ip link set dev {{ l.ifname }} up {% endfor %} -exit 0 \ No newline at end of file +exit 0 From da20885ffe5fec2c067f82eada6d0368d10c53ab Mon Sep 17 00:00:00 2001 From: J vanBemmel Date: Fri, 11 Oct 2024 08:39:38 -0500 Subject: [PATCH 16/21] Only set LACP options in 802.3ad mode --- netsim/ansible/templates/lag/frr.j2 | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/netsim/ansible/templates/lag/frr.j2 b/netsim/ansible/templates/lag/frr.j2 index 5c8bd87fbb..21337d7aa9 100644 --- a/netsim/ansible/templates/lag/frr.j2 +++ b/netsim/ansible/templates/lag/frr.j2 @@ -8,14 +8,14 @@ set -e # Exit immediately when any command fails # {% for l in interfaces if 'lag' in l %} {% if 'parentindex' not in l %} -{% set _lacp = l.lag.lacp|default(lag.lacp) %} -{% set lacp_act = 'off' if _lacp=='off' else 'on' %} -{% set lacp_rate = ('lacp_rate ' + _lacp) if _lacp!='off' else '' %} {% set _m = l.lag.mode|default(lag.mode) %} {% if _m=="802.3ad" %} -{% set _m = _m + " xmit_hash_policy encap3+4" %} +{% set _lacp = l.lag.lacp|default(lag.lacp) %} +{% set lacp_act = 'off' if _lacp=='off' else 'on' %} +{% set lacp_rate = (' lacp_rate ' + _lacp) if _lacp!='off' else '' %} +{% set _m = _m + " xmit_hash_policy encap3+4 lacp_active " + lacp_act + lacp_rate %} {% endif %} -ip link add dev {{l.ifname}} type bond mode {{_m}} lacp_active {{lacp_act}} {{lacp_rate}} || true +ip link add dev {{l.ifname}} type bond mode {{_m}} {% endif %} ip link set dev {{ l.ifname }} down {% endfor %} From b8d994de7bd8c763768385eb4978a495de405119 Mon Sep 17 00:00:00 2001 From: Jeroen van Bemmel Date: Sun, 20 Oct 2024 16:11:17 -0500 Subject: [PATCH 17/21] Refactor lag module to use ```lag.members``` structure instead of link groups Based on discussion, this syntax forces a check for the presence of the lag module --- docs/module/lag.md | 16 +- netsim/ansible/templates/lag/frr.j2 | 4 +- netsim/augment/links.py | 2 - netsim/modules/lag.py | 139 +++++++------ tests/integration/lag/01-l3-lag.yml | 10 +- .../integration/lag/02-l3-lag-with-vlans.yml | 6 +- tests/topology/expected/lag-l3-vlan-trunk.yml | 183 ++++++++++-------- tests/topology/expected/lag-l3.yml | 136 +++++++------ tests/topology/input/lag-l3-vlan-trunk.yml | 6 +- tests/topology/input/lag-l3.yml | 6 +- 10 files changed, 270 insertions(+), 238 deletions(-) diff --git a/docs/module/lag.md b/docs/module/lag.md index 51bd0c60e7..2ef9b00983 100644 --- a/docs/module/lag.md +++ b/docs/module/lag.md @@ -20,11 +20,13 @@ The following parameters can be set globally or per node/link: Note that 'link down' is not easily detectable in a virtual environment with veth pairs, therefore it is strongly recommended to enable LACP whenever possible -* **lacp_mode**: "active" or "passive" +* **lacp_mode**: "active" (default) or "passive"; note that at most 1 node can be passive +The following parameters can be set per link: +* **members**: List or dict of links that form the LAG, mandatory and formatted like **topology.links** * **ifindex**: Optional parameter to control naming of the bonding device -By setting the link type to **lag** for a link group, a *lag* type link is created with the given list of member links. +By creating a link with **lag.members** defined, a *lag* type link is created with the given list of member links. ## Example @@ -36,18 +38,14 @@ module: [ lag ] nodes: [ r1, r2 ] links: -- group: lag1 - type: lag - members: [ r1-r2, r1-r2 ] +- lag.members: [ r1-r2, r1-r2 ] ``` -Additional parameters such as vlan trunks, OSPF cost, etc. can be applied to such *lag* type link groups. +Additional parameters such as vlan trunks, OSPF cost, etc. can be applied to such *lag* type links. In case additional attributes are required for the member links, the members can be expanded: ``` links: -- group: lag1 - type: lag - members: +- lag.members: - r1: ifindex: 49 # Use 100G links 1/1/49 and 1/1/50 r2: diff --git a/netsim/ansible/templates/lag/frr.j2 b/netsim/ansible/templates/lag/frr.j2 index 21337d7aa9..714456847c 100644 --- a/netsim/ansible/templates/lag/frr.j2 +++ b/netsim/ansible/templates/lag/frr.j2 @@ -7,7 +7,7 @@ set -e # Exit immediately when any command fails # Create bonds for LAGs, if any. Requires kernel bonding module loaded # {% for l in interfaces if 'lag' in l %} -{% if 'parentindex' not in l %} +{% if l.type=='lag' %} {% set _m = l.lag.mode|default(lag.mode) %} {% if _m=="802.3ad" %} {% set _lacp = l.lag.lacp|default(lag.lacp) %} @@ -21,7 +21,7 @@ ip link set dev {{ l.ifname }} down {% endfor %} {% for l in interfaces if 'lag' in l %} -{% if 'parentindex' in l %} +{% if l.type=='p2p' %} ip link set dev {{ l.ifname }} master {% for i in interfaces if i.type=='lag' and i.linkindex==l.parentindex %}{{ i.ifname }} {% endfor %} {% endif %} diff --git a/netsim/augment/links.py b/netsim/augment/links.py index c30386f549..5857d71cac 100644 --- a/netsim/augment/links.py +++ b/netsim/augment/links.py @@ -1056,7 +1056,6 @@ def expand_groups(topology: Box) -> None: continue # Report error and skip otherwise copy_group_data = data.get_box(link) # We'll copy all group data into member links - copy_group_data._link_group = link.group # Keep track of link group that members belong to for key in ['group','members','_linkname']: # Apart from the link name and copy_group_data.pop(key,None) @@ -1103,4 +1102,3 @@ def cleanup(topology: Box) -> None: for link in topology.links: link.pop('_linkname',None) - link.pop('_link_group',None) diff --git a/netsim/modules/lag.py b/netsim/modules/lag.py index dce241a77e..7856f71fd4 100644 --- a/netsim/modules/lag.py +++ b/netsim/modules/lag.py @@ -9,78 +9,95 @@ ID_SET = 'lag_id' +""" +populate_lag_id_set -- Collect any user defined lag.ifindex values globally and initialize ID generator +""" def populate_lag_id_set(topology: Box) -> None: _dataplane.create_id_set(ID_SET) LAG_IDS = { l.lag.ifindex for l in topology.links if 'lag' in l and 'ifindex' in l.lag } _dataplane.extend_id_set(ID_SET,LAG_IDS) _dataplane.set_id_counter(ID_SET,topology.defaults.lag.start_lag_id,100) -class LAG(_Module): +""" +check_lag_feature -- Verify that all nodes involved in a LAG link support the feature +""" +def check_lag_feature(memberlink: Box,topology: Box) -> bool: - def module_pre_transform(self, topology: Box) -> None: - populate_lag_id_set(topology) + # Check that lag member links have exactly 2 nodes + if len(memberlink.interfaces)!=2: + log.error( + f'Link {memberlink._linkname} in LAG {memberlink.lag.ifindex} must have exactly 2 nodes', + category=log.IncorrectAttr, + module='lag') + return False - """ - link_pre_transform: Process LAG link groups and add virtual parent links to the topology - """ - def link_pre_transform(self, link: Box, topology: Box) -> None: - if log.debug_active('lag'): - print(f'LAG link_pre_transform for {link}') + ok = True + for i in memberlink.interfaces: + n = topology.nodes[i.node] + features = devices.get_device_features(n,topology.defaults) + if 'lag' not in features: + log.error( + f'Node {n.name} does not support LAG configured on link {memberlink._linkname}', + category=log.IncorrectAttr, + module='lag', + hint='lag') + ok = False + ATT = 'lag.lacp_mode' + lacp_mode = i.get(ATT) or memberlink.get(ATT) or n.get(ATT) or topology.defaults.get(ATT) + if lacp_mode=='passive' and not features.lag.get('passive',False): + log.error( + f'Node {n.name} does not support passive LACP configured on link {memberlink._linkname}', + category=log.IncorrectAttr, + module='lag', + hint='lag') + ok = False + return ok - # Iterate over links with type lag, created for link group(s) - if link.get('type',"")=="lag" and '_link_group' in link: - group_name = link._link_group - # Check that lag member links have exactly 2 nodes - if len(link.interfaces)!=2: - log.error( - f'Links in LAG group {group_name} must have exactly 2 nodes', +""" +create_lag_member_links -- iterate over topology.links and expand any that have lag.members defined +""" +def create_lag_member_links(topology: Box) -> None: + for l in list(topology.links): + if 'lag' in l: + if not 'members' in l.lag: + log.error( + f'must define "lag.members" on link {l._linkname}', category=log.IncorrectAttr, - module='lag', - hint='lag') + module='lag') + l.type = 'lag' - # 1. Check that the 2 nodes involved all (both) support LAG - for i in link.interfaces: - n = topology.nodes[i.node] - features = devices.get_device_features(n,topology.defaults) - if 'lag' not in features: - log.error( - f'Node {n.name} does not support LAG configured on link {link._linkname}', - category=log.IncorrectAttr, - module='lag', - hint='lag') - ATT = 'lag.lacp_mode' - lacp_mode = i.get(ATT) or link.get(ATT) or n.get(ATT) or topology.defaults.get(ATT) - if lacp_mode=='passive' and not features.lag.get('passive',False): - log.error( - f'Node {n.name} does not support passive LACP configured on link {link._linkname}', - category=log.IncorrectAttr, - module='lag', - hint='lag') + if 'ifindex' not in l.lag: # Use user provided lag.ifindex, if any + l.lag.ifindex = _dataplane.get_next_id(ID_SET) + + copy_link_data = data.get_box(l) # We'll copy all link data into member links + copy_link_data.type = "p2p" # Mark as p2p + copy_link_data.prefix = False # Don't allow IP assignment for these links + for k in ['vlan','lag.members']: # Remove lag.members and any VLAN parameters + copy_link_data.pop(k,None) - # Find parent virtual link, create if not existing - parents = [ l for l in topology.links if l.get("type")=="lag" and l._linkname == group_name ] - if not parents: - parent = data.get_box(link) - parent._linkname = group_name - parent.pop('_link_group',None) # Don't include parent in group - parent.linkindex = len(topology.links) + 1 - parent.interfaces = link.interfaces + [] # Deep copy, assumes all links have same 2 nodes - if 'ifindex' not in parent.lag: # Use given lag.ifindex, if any - parent.lag.ifindex = _dataplane.get_next_id(ID_SET) - topology.links.append( parent ) - if log.debug_active('lag'): - print(f'LAG link_pre_transform created virtual parent {parent}') - else: - parent = parents[0] - # For future mc-lag: add any new nodes - # parent.interfaces.extend( [ n for n in link.interfaces if n not in parent.interfaces ] ) + lag_members = l.lag.members + l.lag.pop("members",None) # Remove explicit list of members + for idx,member in enumerate(lag_members): + member = links.adjust_link_object(member,f'lag{l.lag.ifindex}[{idx+1}]',topology.nodes) - # Modify the LAG member link - link.type = "p2p" # Change type back to p2p - link.lag = link.lag or {} # ..and make sure template code can check for lag links - link.prefix = False # Disallow IP addressing - link.pop('vlan',None) # Remove any VLAN, moved to the virtual lag interface - link.parentindex = parent.linkindex # Link physical interface to its virtual parent + # After normalizing, check that all nodes involved support LAGs + if check_lag_feature(member,topology): + member = copy_link_data + member # Copy group data into member link + member.linkindex = len(topology.links)+1 + member.parentindex = l.linkindex # Keep track of parent + if log.debug_active('lag'): + print(f'LAG create_lag_member_links -> adding link {member}') + topology.links.append(member) + + if not l.interfaces: # Copy interfaces from first member link + l.interfaces = member.interfaces + [] # Deep copy, assumes all links have same 2 nodes + +class LAG(_Module): + + def module_pre_transform(self, topology: Box) -> None: + if log.debug_active('lag'): + print(f'LAG module_pre_transform') + populate_lag_id_set(topology) - if log.debug_active('lag'): - print(f'After LAG link_pre_transform: {link}') + # Expand lag.members into additional p2p links + create_lag_member_links(topology) diff --git a/tests/integration/lag/01-l3-lag.yml b/tests/integration/lag/01-l3-lag.yml index 5edd236201..1429eb6134 100644 --- a/tests/integration/lag/01-l3-lag.yml +++ b/tests/integration/lag/01-l3-lag.yml @@ -13,13 +13,9 @@ groups: module: [ lag, ospf ] links: -- group: lag1_2 - type: lag - members: [r1-r2,r1-r2] -- group: lag2_3 - type: lag - lag.ifindex: 0 # Name it as 'bond0' -> First one becomes 'bond1' - members: +- lag.members: [r1-r2,r1-r2] +- lag.ifindex: 0 # Name it as 'bond0' -> First one becomes 'bond1' + lag.members: - r2: ifindex: 3 r3: diff --git a/tests/integration/lag/02-l3-lag-with-vlans.yml b/tests/integration/lag/02-l3-lag-with-vlans.yml index 90cf2d4015..b025cc2b3a 100644 --- a/tests/integration/lag/02-l3-lag-with-vlans.yml +++ b/tests/integration/lag/02-l3-lag-with-vlans.yml @@ -17,10 +17,8 @@ vlans: v2: links: -- group: lag1-with-vlan - type: lag - vlan.trunk: [ v1, v2 ] - members: +- vlan.trunk: [ v1, v2 ] + lag.members: - r1-r2 - r1-r2 - r1: diff --git a/tests/topology/expected/lag-l3-vlan-trunk.yml b/tests/topology/expected/lag-l3-vlan-trunk.yml index 7e0147840d..bbd91bad56 100644 --- a/tests/topology/expected/lag-l3-vlan-trunk.yml +++ b/tests/topology/expected/lag-l3-vlan-trunk.yml @@ -6,6 +6,32 @@ lag: lacp_mode: active mode: 802.3ad links: +- bridge: input_1 + interfaces: + - ifindex: 30000 + ifname: bond0 + node: r1 + vlan: + trunk: + v1: {} + v2: {} + - ifindex: 30000 + ifname: bond0 + node: r2 + vlan: + trunk: + v1: {} + v2: {} + lag: + ifindex: 0 + linkindex: 1 + node_count: 2 + prefix: {} + type: lag + vlan: + trunk: + v1: {} + v2: {} - interfaces: - ifindex: 1 ifname: eth1 @@ -13,10 +39,11 @@ links: - ifindex: 1 ifname: eth1 node: r2 - lag: {} - linkindex: 1 + lag: + ifindex: 0 + linkindex: 2 node_count: 2 - parentindex: 4 + parentindex: 1 prefix: false type: p2p - interfaces: @@ -26,10 +53,11 @@ links: - ifindex: 2 ifname: eth2 node: r2 - lag: {} - linkindex: 2 + lag: + ifindex: 0 + linkindex: 3 node_count: 2 - parentindex: 4 + parentindex: 1 prefix: false type: p2p - interfaces: @@ -39,38 +67,13 @@ links: - ifindex: 3 ifname: eth3 node: r2 - lag: {} - linkindex: 3 - node_count: 2 - parentindex: 4 - prefix: false - type: p2p -- bridge: input_4 - interfaces: - - ifindex: 30000 - ifname: bond0 - node: r1 - vlan: - trunk: - v1: {} - v2: {} - - ifindex: 30000 - ifname: bond0 - node: r2 - vlan: - trunk: - v1: {} - v2: {} lag: ifindex: 0 linkindex: 4 node_count: 2 - prefix: {} - type: lag - vlan: - trunk: - v1: {} - v2: {} + parentindex: 1 + prefix: false + type: p2p module: - lag - vlan @@ -92,57 +95,63 @@ nodes: hostname: clab-input-r1 id: 1 interfaces: + - ifindex: 30000 + ifname: bond0 + lag: + ifindex: 0 + linkindex: 1 + mtu: 1500 + name: r1 -> r2 + neighbors: + - ifname: bond0 + lag: + ifindex: 0 + node: r2 + subif_index: 2 + type: lag + virtual_interface: true - ifindex: 1 ifname: eth1 - lag: {} - linkindex: 1 + lag: + ifindex: 0 + linkindex: 2 mtu: 1500 name: r1 -> r2 neighbors: - ifname: eth1 - lag: {} + lag: + ifindex: 0 node: r2 - parentindex: 4 + parentindex: 1 type: p2p - ifindex: 2 ifname: eth2 - lag: {} - linkindex: 2 + lag: + ifindex: 0 + linkindex: 3 mtu: 1500 name: r1 -> r2 neighbors: - ifname: eth2 - lag: {} + lag: + ifindex: 0 node: r2 - parentindex: 4 + parentindex: 1 type: p2p - ifindex: 3 ifname: eth3 - lag: {} - linkindex: 3 - mtu: 1500 - name: r1 -> r2 - neighbors: - - ifname: eth3 - lag: {} - node: r2 - parentindex: 4 - type: p2p - - ifindex: 30000 - ifname: bond0 lag: ifindex: 0 linkindex: 4 mtu: 1500 name: r1 -> r2 neighbors: - - ifname: bond0 + - ifname: eth3 lag: ifindex: 0 node: r2 - subif_index: 2 - type: lag - virtual_interface: true + parentindex: 1 + type: p2p - ifindex: 4 ifname: bond0.1000 parent_ifindex: 30000 @@ -174,6 +183,7 @@ nodes: virtual_interface: true vlan: mode: irb + name: v1 - bridge_group: 2 ifindex: 7 ifname: vlan1001 @@ -187,6 +197,7 @@ nodes: virtual_interface: true vlan: mode: irb + name: v2 lag: lacp: fast lacp_mode: active @@ -240,57 +251,63 @@ nodes: hostname: clab-input-r2 id: 2 interfaces: + - ifindex: 30000 + ifname: bond0 + lag: + ifindex: 0 + linkindex: 1 + mtu: 1500 + name: r2 -> r1 + neighbors: + - ifname: bond0 + lag: + ifindex: 0 + node: r1 + subif_index: 2 + type: lag + virtual_interface: true - ifindex: 1 ifname: eth1 - lag: {} - linkindex: 1 + lag: + ifindex: 0 + linkindex: 2 mtu: 1500 name: r2 -> r1 neighbors: - ifname: eth1 - lag: {} + lag: + ifindex: 0 node: r1 - parentindex: 4 + parentindex: 1 type: p2p - ifindex: 2 ifname: eth2 - lag: {} - linkindex: 2 + lag: + ifindex: 0 + linkindex: 3 mtu: 1500 name: r2 -> r1 neighbors: - ifname: eth2 - lag: {} + lag: + ifindex: 0 node: r1 - parentindex: 4 + parentindex: 1 type: p2p - ifindex: 3 ifname: eth3 - lag: {} - linkindex: 3 - mtu: 1500 - name: r2 -> r1 - neighbors: - - ifname: eth3 - lag: {} - node: r1 - parentindex: 4 - type: p2p - - ifindex: 30000 - ifname: bond0 lag: ifindex: 0 linkindex: 4 mtu: 1500 name: r2 -> r1 neighbors: - - ifname: bond0 + - ifname: eth3 lag: ifindex: 0 node: r1 - subif_index: 2 - type: lag - virtual_interface: true + parentindex: 1 + type: p2p - ifindex: 4 ifname: bond0.1000 parent_ifindex: 30000 @@ -322,6 +339,7 @@ nodes: virtual_interface: true vlan: mode: irb + name: v1 - bridge_group: 2 ifindex: 7 ifname: vlan1001 @@ -335,6 +353,7 @@ nodes: virtual_interface: true vlan: mode: irb + name: v2 lag: lacp: fast lacp_mode: active diff --git a/tests/topology/expected/lag-l3.yml b/tests/topology/expected/lag-l3.yml index 9379dfbb9f..ee77139f3a 100644 --- a/tests/topology/expected/lag-l3.yml +++ b/tests/topology/expected/lag-l3.yml @@ -6,6 +6,24 @@ lag: lacp_mode: active mode: 802.3ad links: +- bridge: input_1 + interfaces: + - ifindex: 30000 + ifname: bond0 + ipv4: 10.1.0.1/30 + node: r1 + - ifindex: 30000 + ifname: bond0 + ipv4: 10.1.0.2/30 + node: r2 + lag: + ifindex: 0 + linkindex: 1 + mtu: 1600 + node_count: 2 + prefix: + ipv4: 10.1.0.0/30 + type: lag - interfaces: - ifindex: 1 ifname: eth1 @@ -13,11 +31,12 @@ links: - ifindex: 1 ifname: eth1 node: r2 - lag: {} - linkindex: 1 + lag: + ifindex: 0 + linkindex: 2 mtu: 1600 node_count: 2 - parentindex: 3 + parentindex: 1 prefix: false type: p2p - interfaces: @@ -27,31 +46,14 @@ links: - ifindex: 2 ifname: eth2 node: r2 - lag: {} - linkindex: 2 - mtu: 1600 - node_count: 2 - parentindex: 3 - prefix: false - type: p2p -- bridge: input_3 - interfaces: - - ifindex: 30000 - ifname: bond0 - ipv4: 10.1.0.1/30 - node: r1 - - ifindex: 30000 - ifname: bond0 - ipv4: 10.1.0.2/30 - node: r2 lag: ifindex: 0 linkindex: 3 mtu: 1600 node_count: 2 - prefix: - ipv4: 10.1.0.0/30 - type: lag + parentindex: 1 + prefix: false + type: p2p module: - lag name: input @@ -72,46 +74,50 @@ nodes: hostname: clab-input-r1 id: 1 interfaces: - - ifindex: 1 - ifname: eth1 - lag: {} + - ifindex: 30000 + ifname: bond0 + ipv4: 10.1.0.1/30 + lag: + ifindex: 0 linkindex: 1 mtu: 1600 name: r1 -> r2 neighbors: - - ifname: eth1 - lag: {} + - ifname: bond0 + ipv4: 10.1.0.2/30 + lag: + ifindex: 0 node: r2 - parentindex: 3 - type: p2p - - ifindex: 2 - ifname: eth2 - lag: {} + type: lag + virtual_interface: true + - ifindex: 1 + ifname: eth1 + lag: + ifindex: 0 linkindex: 2 mtu: 1600 name: r1 -> r2 neighbors: - - ifname: eth2 - lag: {} + - ifname: eth1 + lag: + ifindex: 0 node: r2 - parentindex: 3 + parentindex: 1 type: p2p - - ifindex: 30000 - ifname: bond0 - ipv4: 10.1.0.1/30 + - ifindex: 2 + ifname: eth2 lag: ifindex: 0 linkindex: 3 mtu: 1600 name: r1 -> r2 neighbors: - - ifname: bond0 - ipv4: 10.1.0.2/30 + - ifname: eth2 lag: ifindex: 0 node: r2 - type: lag - virtual_interface: true + parentindex: 1 + type: p2p lag: lacp: fast lacp_mode: active @@ -147,46 +153,50 @@ nodes: hostname: clab-input-r2 id: 2 interfaces: - - ifindex: 1 - ifname: eth1 - lag: {} + - ifindex: 30000 + ifname: bond0 + ipv4: 10.1.0.2/30 + lag: + ifindex: 0 linkindex: 1 mtu: 1600 name: r2 -> r1 neighbors: - - ifname: eth1 - lag: {} + - ifname: bond0 + ipv4: 10.1.0.1/30 + lag: + ifindex: 0 node: r1 - parentindex: 3 - type: p2p - - ifindex: 2 - ifname: eth2 - lag: {} + type: lag + virtual_interface: true + - ifindex: 1 + ifname: eth1 + lag: + ifindex: 0 linkindex: 2 mtu: 1600 name: r2 -> r1 neighbors: - - ifname: eth2 - lag: {} + - ifname: eth1 + lag: + ifindex: 0 node: r1 - parentindex: 3 + parentindex: 1 type: p2p - - ifindex: 30000 - ifname: bond0 - ipv4: 10.1.0.2/30 + - ifindex: 2 + ifname: eth2 lag: ifindex: 0 linkindex: 3 mtu: 1600 name: r2 -> r1 neighbors: - - ifname: bond0 - ipv4: 10.1.0.1/30 + - ifname: eth2 lag: ifindex: 0 node: r1 - type: lag - virtual_interface: true + parentindex: 1 + type: p2p lag: lacp: fast lacp_mode: active diff --git a/tests/topology/input/lag-l3-vlan-trunk.yml b/tests/topology/input/lag-l3-vlan-trunk.yml index 152369fcee..99c2578b3b 100644 --- a/tests/topology/input/lag-l3-vlan-trunk.yml +++ b/tests/topology/input/lag-l3-vlan-trunk.yml @@ -14,7 +14,5 @@ vlans: nodes: [ r1, r2 ] links: -- group: lag_with_trunk - type: lag - vlan.trunk: [v1,v2] - members: [ r1-r2, r1-r2, r1-r2 ] +- vlan.trunk: [v1,v2] + lag.members: [ r1-r2, r1-r2, r1-r2 ] diff --git a/tests/topology/input/lag-l3.yml b/tests/topology/input/lag-l3.yml index 0e27f0b0c1..1dffdb07c6 100644 --- a/tests/topology/input/lag-l3.yml +++ b/tests/topology/input/lag-l3.yml @@ -9,7 +9,5 @@ defaults: module: [ lag ] nodes: [ r1, r2 ] links: -- group: lag1 - type: lag - mtu: 1600 # Test that MTU is copied to member links - members: [ r1-r2, r1-r2 ] +- mtu: 1600 # Test that MTU is copied to member links + lag.members: [ r1-r2, r1-r2 ] From 65ee6c2b32d3d193237e1a7e60b5b29c213d1d44 Mon Sep 17 00:00:00 2001 From: Jeroen van Bemmel Date: Thu, 24 Oct 2024 18:04:46 -0500 Subject: [PATCH 18/21] Updated after feedback --- netsim/modules/lag.py | 135 ++++++++++++++-------------- netsim/modules/lag.yml | 13 +-- tests/integration/lag/01-l3-lag.yml | 2 +- 3 files changed, 78 insertions(+), 72 deletions(-) diff --git a/netsim/modules/lag.py b/netsim/modules/lag.py index 7856f71fd4..9e1be71eea 100644 --- a/netsim/modules/lag.py +++ b/netsim/modules/lag.py @@ -14,83 +14,68 @@ """ def populate_lag_id_set(topology: Box) -> None: _dataplane.create_id_set(ID_SET) - LAG_IDS = { l.lag.ifindex for l in topology.links if 'lag' in l and 'ifindex' in l.lag } + LAG_IDS = { l.lag.ifindex for l in topology.links if l.get('lag.ifindex',None) } _dataplane.extend_id_set(ID_SET,LAG_IDS) _dataplane.set_id_counter(ID_SET,topology.defaults.lag.start_lag_id,100) -""" -check_lag_feature -- Verify that all nodes involved in a LAG link support the feature -""" -def check_lag_feature(memberlink: Box,topology: Box) -> bool: - - # Check that lag member links have exactly 2 nodes - if len(memberlink.interfaces)!=2: - log.error( - f'Link {memberlink._linkname} in LAG {memberlink.lag.ifindex} must have exactly 2 nodes', - category=log.IncorrectAttr, - module='lag') - return False - - ok = True - for i in memberlink.interfaces: - n = topology.nodes[i.node] - features = devices.get_device_features(n,topology.defaults) - if 'lag' not in features: - log.error( - f'Node {n.name} does not support LAG configured on link {memberlink._linkname}', - category=log.IncorrectAttr, - module='lag', - hint='lag') - ok = False - ATT = 'lag.lacp_mode' - lacp_mode = i.get(ATT) or memberlink.get(ATT) or n.get(ATT) or topology.defaults.get(ATT) - if lacp_mode=='passive' and not features.lag.get('passive',False): - log.error( - f'Node {n.name} does not support passive LACP configured on link {memberlink._linkname}', - category=log.IncorrectAttr, - module='lag', - hint='lag') - ok = False - return ok - """ create_lag_member_links -- iterate over topology.links and expand any that have lag.members defined """ def create_lag_member_links(topology: Box) -> None: for l in list(topology.links): - if 'lag' in l: - if not 'members' in l.lag: - log.error( - f'must define "lag.members" on link {l._linkname}', - category=log.IncorrectAttr, - module='lag') - l.type = 'lag' - - if 'ifindex' not in l.lag: # Use user provided lag.ifindex, if any - l.lag.ifindex = _dataplane.get_next_id(ID_SET) - - copy_link_data = data.get_box(l) # We'll copy all link data into member links - copy_link_data.type = "p2p" # Mark as p2p - copy_link_data.prefix = False # Don't allow IP assignment for these links - for k in ['vlan','lag.members']: # Remove lag.members and any VLAN parameters - copy_link_data.pop(k,None) + if 'lag' not in l: + continue + elif not 'members' in l.lag: + log.error(f'must define "lag.members" on LAG link {l._linkname}', + category=log.IncorrectAttr, + module='lag') + l.type = 'lag' + if 'ifindex' not in l.lag: # Use user provided lag.ifindex, if any + l.lag.ifindex = _dataplane.get_next_id(ID_SET) + l2_ifdata = { 'type': "p2p", 'prefix': False } # Construct an L2 member link + for a in list(topology.defaults.lag.attributes.lag_l2_ifattr): + if a in l: + l2_ifdata[a] = l[a] - lag_members = l.lag.members - l.lag.pop("members",None) # Remove explicit list of members - for idx,member in enumerate(lag_members): - member = links.adjust_link_object(member,f'lag{l.lag.ifindex}[{idx+1}]',topology.nodes) + lag_members = l.lag.members + l.lag.pop("members",None) # Remove explicit list of members + for idx,member in enumerate(lag_members): + member = links.adjust_link_object(member,f'lag{l.lag.ifindex}[{idx+1}]',topology.nodes) - # After normalizing, check that all nodes involved support LAGs - if check_lag_feature(member,topology): - member = copy_link_data + member # Copy group data into member link - member.linkindex = len(topology.links)+1 - member.parentindex = l.linkindex # Keep track of parent - if log.debug_active('lag'): - print(f'LAG create_lag_member_links -> adding link {member}') - topology.links.append(member) + if len(member.interfaces)!=2: # Check that there are exactly 2 nodes involved + log.error(f'Link {member._linkname} in LAG {l.lag.ifindex} must have exactly 2 nodes', + category=log.IncorrectAttr, + module='lag') + else: # Check that they all support LAG + for i in member.interfaces: + _n = topology.nodes[i.node] + features = devices.get_device_features(_n,topology.defaults) + if 'lag' not in features: + log.error(f'Node {_n.name} ({_n.device}) does not support lag module, cannot be part of LAG {member._linkname}', + category=log.IncorrectAttr, + module='lag') + return + if 'lag' not in _n.get('module',[]): + log.error(f'lag module not enabled for node {_n.name}, cannot be part of LAG {member._linkname}', + category=log.IncorrectAttr, + module='lag') + return - if not l.interfaces: # Copy interfaces from first member link - l.interfaces = member.interfaces + [] # Deep copy, assumes all links have same 2 nodes + member = l2_ifdata + member # Copy L2 data into member link + member.linkindex = len(topology.links)+1 + member.parentindex = l.linkindex # Keep track of parent + if log.debug_active('lag'): + print(f'LAG create_lag_member_links -> adding link {member}') + topology.links.append(member) + if not l.interfaces: # Copy interfaces from first member link + l.interfaces = member.interfaces + [] # Deep copy, assumes all links have same 2 nodes + else: + base = { n.node for n in l.interfaces } # List the (2) nodes from the first link + others = { n.node for n in member.interfaces if n.node not in base } + if others: + log.error(f'All LAG link members must connect the same pair of nodes({base}), found {others}', + category=log.IncorrectAttr, + module='lag') class LAG(_Module): @@ -101,3 +86,21 @@ def module_pre_transform(self, topology: Box) -> None: # Expand lag.members into additional p2p links create_lag_member_links(topology) + + """ + After attribute propagation and consolidation, verify that requested features are supported + + Only gets called for nodes with 'lag' module enabled + """ + def node_post_transform(self, node: Box, topology: Box) -> None: + features = devices.get_device_features(node,topology.defaults) + for i in node.interfaces: + if 'lag' not in i: + continue + ATT = 'lag.lacp_mode' + lacp_mode = i.get(ATT) or node.get(ATT) or topology.defaults.get(ATT) + if lacp_mode=='passive' and not features.lag.get('passive',False): + log.error(f'Node {node.name} does not support passive LACP configured on interface {i.ifname}', + category=log.IncorrectAttr, + module='lag', + hint='lag') diff --git a/netsim/modules/lag.yml b/netsim/modules/lag.yml index ca88794940..7783622d84 100644 --- a/netsim/modules/lag.yml +++ b/netsim/modules/lag.yml @@ -17,10 +17,13 @@ attributes: lacp_mode: mode: { type: str, valid_values: [ "802.3ad", "balance-xor", "active-backup" ] } link: # Most should be consistent across both interfaces on the link - lacp: - lacp_mode: - ifindex: int # Optional, to control naming of the bonding interface + lacp: { copy: global } + lacp_mode: { copy: global } + ifindex: { type: int, min_value: 0, max_value: 10000 } # Optional, to control naming of the bonding interface members: mode: { type: str, valid_values: [ "802.3ad", "balance-xor", "active-backup" ] } - interface: - lacp_mode: + + # Copy these L2 attributes into LAG interfaces + lag_l2_ifattr: + lag.ifindex: + mtu: diff --git a/tests/integration/lag/01-l3-lag.yml b/tests/integration/lag/01-l3-lag.yml index 1429eb6134..feed428235 100644 --- a/tests/integration/lag/01-l3-lag.yml +++ b/tests/integration/lag/01-l3-lag.yml @@ -33,4 +33,4 @@ validate: nodes: [ h1 ] wait_msg: Waiting for STP to enable the ports wait: 45 - plugin: ping('h2') \ No newline at end of file + plugin: ping('h2') From b775c8a5beff1867227f86dc3dd87e460da577df Mon Sep 17 00:00:00 2001 From: Jeroen van Bemmel Date: Thu, 24 Oct 2024 18:20:29 -0500 Subject: [PATCH 19/21] Update test results, allow lag.ifindex==0 values too --- netsim/modules/lag.py | 3 ++- netsim/modules/lag.yml | 2 +- tests/topology/expected/lag-l3-vlan-trunk.yml | 16 ---------------- tests/topology/expected/lag-l3.yml | 12 ------------ 4 files changed, 3 insertions(+), 30 deletions(-) diff --git a/netsim/modules/lag.py b/netsim/modules/lag.py index 9e1be71eea..63fd5620d4 100644 --- a/netsim/modules/lag.py +++ b/netsim/modules/lag.py @@ -14,7 +14,8 @@ """ def populate_lag_id_set(topology: Box) -> None: _dataplane.create_id_set(ID_SET) - LAG_IDS = { l.lag.ifindex for l in topology.links if l.get('lag.ifindex',None) } + # Note that 0 is a valid lag.ifindex value + LAG_IDS = { l.lag.ifindex for l in topology.links if 'lag' in l and 'ifindex' in l.lag } _dataplane.extend_id_set(ID_SET,LAG_IDS) _dataplane.set_id_counter(ID_SET,topology.defaults.lag.start_lag_id,100) diff --git a/netsim/modules/lag.yml b/netsim/modules/lag.yml index 7783622d84..42cf519626 100644 --- a/netsim/modules/lag.yml +++ b/netsim/modules/lag.yml @@ -23,7 +23,7 @@ attributes: members: mode: { type: str, valid_values: [ "802.3ad", "balance-xor", "active-backup" ] } - # Copy these L2 attributes into LAG interfaces + # Copy only these L2 attributes into LAG physical link members lag_l2_ifattr: lag.ifindex: mtu: diff --git a/tests/topology/expected/lag-l3-vlan-trunk.yml b/tests/topology/expected/lag-l3-vlan-trunk.yml index bbd91bad56..5858dd33fd 100644 --- a/tests/topology/expected/lag-l3-vlan-trunk.yml +++ b/tests/topology/expected/lag-l3-vlan-trunk.yml @@ -104,8 +104,6 @@ nodes: name: r1 -> r2 neighbors: - ifname: bond0 - lag: - ifindex: 0 node: r2 subif_index: 2 type: lag @@ -119,8 +117,6 @@ nodes: name: r1 -> r2 neighbors: - ifname: eth1 - lag: - ifindex: 0 node: r2 parentindex: 1 type: p2p @@ -133,8 +129,6 @@ nodes: name: r1 -> r2 neighbors: - ifname: eth2 - lag: - ifindex: 0 node: r2 parentindex: 1 type: p2p @@ -147,8 +141,6 @@ nodes: name: r1 -> r2 neighbors: - ifname: eth3 - lag: - ifindex: 0 node: r2 parentindex: 1 type: p2p @@ -260,8 +252,6 @@ nodes: name: r2 -> r1 neighbors: - ifname: bond0 - lag: - ifindex: 0 node: r1 subif_index: 2 type: lag @@ -275,8 +265,6 @@ nodes: name: r2 -> r1 neighbors: - ifname: eth1 - lag: - ifindex: 0 node: r1 parentindex: 1 type: p2p @@ -289,8 +277,6 @@ nodes: name: r2 -> r1 neighbors: - ifname: eth2 - lag: - ifindex: 0 node: r1 parentindex: 1 type: p2p @@ -303,8 +289,6 @@ nodes: name: r2 -> r1 neighbors: - ifname: eth3 - lag: - ifindex: 0 node: r1 parentindex: 1 type: p2p diff --git a/tests/topology/expected/lag-l3.yml b/tests/topology/expected/lag-l3.yml index ee77139f3a..c277bc8115 100644 --- a/tests/topology/expected/lag-l3.yml +++ b/tests/topology/expected/lag-l3.yml @@ -85,8 +85,6 @@ nodes: neighbors: - ifname: bond0 ipv4: 10.1.0.2/30 - lag: - ifindex: 0 node: r2 type: lag virtual_interface: true @@ -99,8 +97,6 @@ nodes: name: r1 -> r2 neighbors: - ifname: eth1 - lag: - ifindex: 0 node: r2 parentindex: 1 type: p2p @@ -113,8 +109,6 @@ nodes: name: r1 -> r2 neighbors: - ifname: eth2 - lag: - ifindex: 0 node: r2 parentindex: 1 type: p2p @@ -164,8 +158,6 @@ nodes: neighbors: - ifname: bond0 ipv4: 10.1.0.1/30 - lag: - ifindex: 0 node: r1 type: lag virtual_interface: true @@ -178,8 +170,6 @@ nodes: name: r2 -> r1 neighbors: - ifname: eth1 - lag: - ifindex: 0 node: r1 parentindex: 1 type: p2p @@ -192,8 +182,6 @@ nodes: name: r2 -> r1 neighbors: - ifname: eth2 - lag: - ifindex: 0 node: r1 parentindex: 1 type: p2p From b8743297b1897757379a8b5d1c77542861b03217 Mon Sep 17 00:00:00 2001 From: Jeroen van Bemmel Date: Thu, 24 Oct 2024 18:41:10 -0500 Subject: [PATCH 20/21] * Propagate attributes * Check that link.members is a list * Change naming of links * Update test results --- docs/module/lag.md | 2 +- netsim/modules/lag.py | 10 +++- netsim/modules/lag.yml | 2 + tests/topology/expected/lag-l3-vlan-trunk.yml | 56 +++++++++++++++++++ tests/topology/expected/lag-l3.yml | 18 ++++++ 5 files changed, 84 insertions(+), 4 deletions(-) diff --git a/docs/module/lag.md b/docs/module/lag.md index 2ef9b00983..76f132d7d5 100644 --- a/docs/module/lag.md +++ b/docs/module/lag.md @@ -23,7 +23,7 @@ The following parameters can be set globally or per node/link: * **lacp_mode**: "active" (default) or "passive"; note that at most 1 node can be passive The following parameters can be set per link: -* **members**: List or dict of links that form the LAG, mandatory and formatted like **topology.links** +* **members**: List of links that form the LAG, mandatory and formatted like **topology.links** * **ifindex**: Optional parameter to control naming of the bonding device By creating a link with **lag.members** defined, a *lag* type link is created with the given list of member links. diff --git a/netsim/modules/lag.py b/netsim/modules/lag.py index 63fd5620d4..b77441d5fb 100644 --- a/netsim/modules/lag.py +++ b/netsim/modules/lag.py @@ -4,6 +4,7 @@ from box import Box, BoxList from . import _Module, _dataplane from .. import data +from ..data import types as _types from ..utils import log from ..augment import devices, links @@ -30,6 +31,9 @@ def create_lag_member_links(topology: Box) -> None: log.error(f'must define "lag.members" on LAG link {l._linkname}', category=log.IncorrectAttr, module='lag') + elif not _types.must_be_list(parent=l.lag,key='members',path=l._linkname,module='lag'): + return + l.type = 'lag' if 'ifindex' not in l.lag: # Use user provided lag.ifindex, if any l.lag.ifindex = _dataplane.get_next_id(ID_SET) @@ -41,7 +45,7 @@ def create_lag_member_links(topology: Box) -> None: lag_members = l.lag.members l.lag.pop("members",None) # Remove explicit list of members for idx,member in enumerate(lag_members): - member = links.adjust_link_object(member,f'lag{l.lag.ifindex}[{idx+1}]',topology.nodes) + member = links.adjust_link_object(member,f'{l._linkname}.lag[{idx+1}]',topology.nodes) if len(member.interfaces)!=2: # Check that there are exactly 2 nodes involved log.error(f'Link {member._linkname} in LAG {l.lag.ifindex} must have exactly 2 nodes', @@ -98,8 +102,8 @@ def node_post_transform(self, node: Box, topology: Box) -> None: for i in node.interfaces: if 'lag' not in i: continue - ATT = 'lag.lacp_mode' - lacp_mode = i.get(ATT) or node.get(ATT) or topology.defaults.get(ATT) + + lacp_mode = i.get('lag.lacp_mode') # Inheritance copying is done elsewhere if lacp_mode=='passive' and not features.lag.get('passive',False): log.error(f'Node {node.name} does not support passive LACP configured on interface {i.ifname}', category=log.IncorrectAttr, diff --git a/netsim/modules/lag.yml b/netsim/modules/lag.yml index 42cf519626..20566e9f4e 100644 --- a/netsim/modules/lag.yml +++ b/netsim/modules/lag.yml @@ -23,6 +23,8 @@ attributes: members: mode: { type: str, valid_values: [ "802.3ad", "balance-xor", "active-backup" ] } + node_copy: [lacp,lacp_mode,mode] + # Copy only these L2 attributes into LAG physical link members lag_l2_ifattr: lag.ifindex: diff --git a/tests/topology/expected/lag-l3-vlan-trunk.yml b/tests/topology/expected/lag-l3-vlan-trunk.yml index 5858dd33fd..d6af16bb08 100644 --- a/tests/topology/expected/lag-l3-vlan-trunk.yml +++ b/tests/topology/expected/lag-l3-vlan-trunk.yml @@ -99,6 +99,9 @@ nodes: ifname: bond0 lag: ifindex: 0 + lacp: fast + lacp_mode: active + mode: 802.3ad linkindex: 1 mtu: 1500 name: r1 -> r2 @@ -112,6 +115,9 @@ nodes: ifname: eth1 lag: ifindex: 0 + lacp: fast + lacp_mode: active + mode: 802.3ad linkindex: 2 mtu: 1500 name: r1 -> r2 @@ -124,6 +130,9 @@ nodes: ifname: eth2 lag: ifindex: 0 + lacp: fast + lacp_mode: active + mode: 802.3ad linkindex: 3 mtu: 1500 name: r1 -> r2 @@ -136,6 +145,9 @@ nodes: ifname: eth3 lag: ifindex: 0 + lacp: fast + lacp_mode: active + mode: 802.3ad linkindex: 4 mtu: 1500 name: r1 -> r2 @@ -146,6 +158,10 @@ nodes: type: p2p - ifindex: 4 ifname: bond0.1000 + lag: + lacp: fast + lacp_mode: active + mode: 802.3ad parent_ifindex: 30000 parent_ifname: bond0 type: vlan_member @@ -155,6 +171,10 @@ nodes: access_id: 1000 - ifindex: 5 ifname: bond0.1001 + lag: + lacp: fast + lacp_mode: active + mode: 802.3ad parent_ifindex: 30000 parent_ifname: bond0 type: vlan_member @@ -166,6 +186,10 @@ nodes: ifindex: 6 ifname: vlan1000 ipv4: 172.16.0.1/24 + lag: + lacp: fast + lacp_mode: active + mode: 802.3ad name: VLAN v1 (1000) -> [r2] neighbors: - ifname: vlan1000 @@ -180,6 +204,10 @@ nodes: ifindex: 7 ifname: vlan1001 ipv4: 172.16.1.1/24 + lag: + lacp: fast + lacp_mode: active + mode: 802.3ad name: VLAN v2 (1001) -> [r2] neighbors: - ifname: vlan1001 @@ -247,6 +275,9 @@ nodes: ifname: bond0 lag: ifindex: 0 + lacp: fast + lacp_mode: active + mode: 802.3ad linkindex: 1 mtu: 1500 name: r2 -> r1 @@ -260,6 +291,9 @@ nodes: ifname: eth1 lag: ifindex: 0 + lacp: fast + lacp_mode: active + mode: 802.3ad linkindex: 2 mtu: 1500 name: r2 -> r1 @@ -272,6 +306,9 @@ nodes: ifname: eth2 lag: ifindex: 0 + lacp: fast + lacp_mode: active + mode: 802.3ad linkindex: 3 mtu: 1500 name: r2 -> r1 @@ -284,6 +321,9 @@ nodes: ifname: eth3 lag: ifindex: 0 + lacp: fast + lacp_mode: active + mode: 802.3ad linkindex: 4 mtu: 1500 name: r2 -> r1 @@ -294,6 +334,10 @@ nodes: type: p2p - ifindex: 4 ifname: bond0.1000 + lag: + lacp: fast + lacp_mode: active + mode: 802.3ad parent_ifindex: 30000 parent_ifname: bond0 type: vlan_member @@ -303,6 +347,10 @@ nodes: access_id: 1000 - ifindex: 5 ifname: bond0.1001 + lag: + lacp: fast + lacp_mode: active + mode: 802.3ad parent_ifindex: 30000 parent_ifname: bond0 type: vlan_member @@ -314,6 +362,10 @@ nodes: ifindex: 6 ifname: vlan1000 ipv4: 172.16.0.2/24 + lag: + lacp: fast + lacp_mode: active + mode: 802.3ad name: VLAN v1 (1000) -> [r1] neighbors: - ifname: vlan1000 @@ -328,6 +380,10 @@ nodes: ifindex: 7 ifname: vlan1001 ipv4: 172.16.1.2/24 + lag: + lacp: fast + lacp_mode: active + mode: 802.3ad name: VLAN v2 (1001) -> [r1] neighbors: - ifname: vlan1001 diff --git a/tests/topology/expected/lag-l3.yml b/tests/topology/expected/lag-l3.yml index c277bc8115..3129279126 100644 --- a/tests/topology/expected/lag-l3.yml +++ b/tests/topology/expected/lag-l3.yml @@ -79,6 +79,9 @@ nodes: ipv4: 10.1.0.1/30 lag: ifindex: 0 + lacp: fast + lacp_mode: active + mode: 802.3ad linkindex: 1 mtu: 1600 name: r1 -> r2 @@ -92,6 +95,9 @@ nodes: ifname: eth1 lag: ifindex: 0 + lacp: fast + lacp_mode: active + mode: 802.3ad linkindex: 2 mtu: 1600 name: r1 -> r2 @@ -104,6 +110,9 @@ nodes: ifname: eth2 lag: ifindex: 0 + lacp: fast + lacp_mode: active + mode: 802.3ad linkindex: 3 mtu: 1600 name: r1 -> r2 @@ -152,6 +161,9 @@ nodes: ipv4: 10.1.0.2/30 lag: ifindex: 0 + lacp: fast + lacp_mode: active + mode: 802.3ad linkindex: 1 mtu: 1600 name: r2 -> r1 @@ -165,6 +177,9 @@ nodes: ifname: eth1 lag: ifindex: 0 + lacp: fast + lacp_mode: active + mode: 802.3ad linkindex: 2 mtu: 1600 name: r2 -> r1 @@ -177,6 +192,9 @@ nodes: ifname: eth2 lag: ifindex: 0 + lacp: fast + lacp_mode: active + mode: 802.3ad linkindex: 3 mtu: 1600 name: r2 -> r1 From 950b4a265cc10397b1c71f055dbefab0733c77de Mon Sep 17 00:00:00 2001 From: Ivan Pepelnjak Date: Fri, 25 Oct 2024 18:28:46 +0200 Subject: [PATCH 21/21] Split the LAG member creation logic into smaller blocks --- netsim/modules/lag.py | 123 ++++++++++++++++++++++-------------------- 1 file changed, 65 insertions(+), 58 deletions(-) diff --git a/netsim/modules/lag.py b/netsim/modules/lag.py index b77441d5fb..aa5a4f00f2 100644 --- a/netsim/modules/lag.py +++ b/netsim/modules/lag.py @@ -16,71 +16,78 @@ def populate_lag_id_set(topology: Box) -> None: _dataplane.create_id_set(ID_SET) # Note that 0 is a valid lag.ifindex value - LAG_IDS = { l.lag.ifindex for l in topology.links if 'lag' in l and 'ifindex' in l.lag } + LAG_IDS = { l.lag.ifindex for l in topology.links + if isinstance(l.get('lag.ifindex',None),int) } _dataplane.extend_id_set(ID_SET,LAG_IDS) _dataplane.set_id_counter(ID_SET,topology.defaults.lag.start_lag_id,100) """ create_lag_member_links -- iterate over topology.links and expand any that have lag.members defined """ -def create_lag_member_links(topology: Box) -> None: - for l in list(topology.links): - if 'lag' not in l: - continue - elif not 'members' in l.lag: - log.error(f'must define "lag.members" on LAG link {l._linkname}', - category=log.IncorrectAttr, - module='lag') - elif not _types.must_be_list(parent=l.lag,key='members',path=l._linkname,module='lag'): - return - - l.type = 'lag' - if 'ifindex' not in l.lag: # Use user provided lag.ifindex, if any - l.lag.ifindex = _dataplane.get_next_id(ID_SET) - l2_ifdata = { 'type': "p2p", 'prefix': False } # Construct an L2 member link - for a in list(topology.defaults.lag.attributes.lag_l2_ifattr): - if a in l: - l2_ifdata[a] = l[a] - - lag_members = l.lag.members - l.lag.pop("members",None) # Remove explicit list of members - for idx,member in enumerate(lag_members): - member = links.adjust_link_object(member,f'{l._linkname}.lag[{idx+1}]',topology.nodes) - - if len(member.interfaces)!=2: # Check that there are exactly 2 nodes involved - log.error(f'Link {member._linkname} in LAG {l.lag.ifindex} must have exactly 2 nodes', +def create_lag_member_links(l: Box, topology: Box) -> None: + lag_members = l.lag.members + l.lag.pop("members",None) # Remove explicit list of members + l2_ifdata = { 'type': "p2p", 'prefix': False } # Construct an L2 member link + for a in list(topology.defaults.lag.attributes.lag_l2_ifattr): + if a in l: + l2_ifdata[a] = l[a] + + for idx,member in enumerate(lag_members): + member = links.adjust_link_object(member,f'{l._linkname}.lag[{idx+1}]',topology.nodes) + + if len(member.interfaces)!=2: # Check that there are exactly 2 nodes involved + log.error(f'Link {member._linkname} in LAG {l.lag.ifindex} must have exactly 2 nodes', + category=log.IncorrectAttr, + module='lag') + return + else: # Check that they all support LAG + for i in member.interfaces: + _n = topology.nodes[i.node] + features = devices.get_device_features(_n,topology.defaults) + if 'lag' not in features: + log.error(f'Node {_n.name} ({_n.device}) does not support lag module, cannot be part of LAG {member._linkname}', + category=log.IncorrectAttr, + module='lag') + return + if 'lag' not in _n.get('module',[]): + log.error(f'lag module not enabled for node {_n.name}, cannot be part of LAG {member._linkname}', + category=log.IncorrectAttr, + module='lag') + return + + member = l2_ifdata + member # Copy L2 data into member link + member.linkindex = len(topology.links)+1 + member.parentindex = l.linkindex # Keep track of parent + if log.debug_active('lag'): + print(f'LAG create_lag_member_links -> adding link {member}') + topology.links.append(member) + if not l.interfaces: # Copy interfaces from first member link + l.interfaces = member.interfaces + [] # Deep copy, assumes all links have same 2 nodes + else: + base = { n.node for n in l.interfaces } # List the (2) nodes from the first link + others = { n.node for n in member.interfaces if n.node not in base } + if others: + log.error(f'All LAG link members must connect the same pair of nodes({base}), found {others}', category=log.IncorrectAttr, module='lag') - else: # Check that they all support LAG - for i in member.interfaces: - _n = topology.nodes[i.node] - features = devices.get_device_features(_n,topology.defaults) - if 'lag' not in features: - log.error(f'Node {_n.name} ({_n.device}) does not support lag module, cannot be part of LAG {member._linkname}', - category=log.IncorrectAttr, - module='lag') - return - if 'lag' not in _n.get('module',[]): - log.error(f'lag module not enabled for node {_n.name}, cannot be part of LAG {member._linkname}', - category=log.IncorrectAttr, - module='lag') - return - - member = l2_ifdata + member # Copy L2 data into member link - member.linkindex = len(topology.links)+1 - member.parentindex = l.linkindex # Keep track of parent - if log.debug_active('lag'): - print(f'LAG create_lag_member_links -> adding link {member}') - topology.links.append(member) - if not l.interfaces: # Copy interfaces from first member link - l.interfaces = member.interfaces + [] # Deep copy, assumes all links have same 2 nodes - else: - base = { n.node for n in l.interfaces } # List the (2) nodes from the first link - others = { n.node for n in member.interfaces if n.node not in base } - if others: - log.error(f'All LAG link members must connect the same pair of nodes({base}), found {others}', - category=log.IncorrectAttr, - module='lag') + +def process_lag_links(topology: Box) -> None: + for l in list(topology.links): + if 'lag' not in l: + continue + elif not 'members' in l.lag: + log.error(f'must define "lag.members" on LAG link {l._linkname}', + category=log.IncorrectAttr, + module='lag') + continue + elif not _types.must_be_list(parent=l.lag,key='members',path=l._linkname,module='lag'): + continue + + l.type = 'lag' + if 'ifindex' not in l.lag: # Use user provided lag.ifindex, if any + l.lag.ifindex = _dataplane.get_next_id(ID_SET) + + create_lag_member_links(l,topology) class LAG(_Module): @@ -90,7 +97,7 @@ def module_pre_transform(self, topology: Box) -> None: populate_lag_id_set(topology) # Expand lag.members into additional p2p links - create_lag_member_links(topology) + process_lag_links(topology) """ After attribute propagation and consolidation, verify that requested features are supported