Lag fixes & cleanup - Githubissues

jbemmel commented 1 week ago

In preparation for a PR adding MLAG support, factoring out these fixes that are not specific to MLAG:

Cleanup lag attributes for member interfaces, only include lag._parentindex and defer to the parent
Disallow setting lag attributes on member interfaces; although some platforms like OS10 could support setting a different LACP speed on different members, other platforms don't support this
Stop VLAN module from removing the virtual_interface and lag attribute from lag links; include regression test case
Check both L3 and L2-only lag cases

ipspace commented 5 days ago

@ssasso: asking for your opinion as this PR involves what seems to be dubious OS10 stuff to me -- does OS10 support routed LAG links (physical interfaces in a LAG, with IP address on a port channel interface), or does it really need VLANs to make LAG work?

ssasso commented 5 days ago

@ssasso: asking for your opinion as this PR involves what seems to be dubious OS10 stuff to me -- does OS10 support routed LAG links (physical interfaces in a LAG, with IP address on a port channel interface), or does it really need VLANs to make LAG work?

r2# configure terminal
r2(config)# interface port-channel 3
r2(conf-if-po-3)# interface range ethernet 1/1/3-1/1/4
r2(conf-range-eth1/1/3-1/1/4)# channel-group 3 mode active
r2(conf-range-eth1/1/3-1/1/4)# interface port-channel 3
r2(conf-if-po-3)# no switchport
r2(conf-if-po-3)# ip address 10.20.30.2/24
r2(conf-if-po-3)# no shutdown
r2(conf-if-po-3)# end
r2# ping 10.20.30.1
PING 10.20.30.1 (10.20.30.1) 56(84) bytes of data.
64 bytes from 10.20.30.1: icmp_seq=1 ttl=64 time=2027 ms
64 bytes from 10.20.30.1: icmp_seq=2 ttl=64 time=1000 ms
64 bytes from 10.20.30.1: icmp_seq=3 ttl=64 time=1.54 ms
64 bytes from 10.20.30.1: icmp_seq=4 ttl=64 time=1.08 ms
64 bytes from 10.20.30.1: icmp_seq=5 ttl=64 time=1.41 ms
64 bytes from 10.20.30.1: icmp_seq=6 ttl=64 time=1.05 ms

--- 10.20.30.1 ping statistics ---
6 packets transmitted, 6 received, 0% packet loss, time 37ms
rtt min/avg/max/mdev = 1.046/505.445/2027.150/772.161 ms, pipe 2

ipspace commented 5 days ago

@ssasso: Thank you ;)

@jbemmel: As you can see, there is no need to require vlan module for OS10 LAG to work ;))

jbemmel commented 5 days ago

@ssasso: Thank you ;)

@jbemmel: As you can see, there is no need to require vlan module for OS10 LAG to work ;))

How I got there: FRR needs the lag module configured before VLANs, as VLANs are created on top of bond interfaces. However, OS10 needs the proper switchport settings before it allows you to configure certain lag settings, and those are configured here https://github.com/ipspace/netlab/blob/e5f6268db24f70a5d0abaa5564ee5fa30a21068e/netsim/ansible/templates/vlan/dellos10.j2#L17

I now realize a better solution is to have the lag module configure the switchport settings it needs, and remove the requires

jbemmel commented 4 days ago

Right now, I see a number of changed test results. I don't know how you got there, but the "virtual_interface" setting must be kept on the VLAN interface and VLAN subinterfaces, or some platforms try to configure "no switchport" on those interfaces and that fails.

Unfortunately I cannot merge this PR until that is fixed. Typically, a LAG PR should not cause a change in any non-LAG test results.

It's due to this - I'll put it back

ipspace / netlab

Lag fixes & cleanup #1541