ESI-LAG: Initial draft implementation by ssasso · Pull Request #2425 · ipspace/netlab

ssasso · 2025-06-24T10:11:04Z

Initial draft implementation of ESI-LAG in LAG module.

I tried to implement this as a plugin first, but it was really a mess to have to interact with the lag module (especially for mlag checks), so I decided to "embed" the code directly in the module itself.

JunOS implementation works, it should be straightforward to add support for FRR and EOS - and I can do it.

But first I'd like your opinion on this @ipspace @jbemmel , then I can also update the documentation as well.

With the current LAG module "syntax", ESI-LAG must be specified in this way:

links:
  sw1_edge:
  - lag:
      members:
      - sw1a:
          vlan.access: rosso
          lag:
            lacp_system_id: 1
            esi: true
        rosso1:
      - sw1b:
          vlan.access: rosso
          lag:
            lacp_system_id: 1
            esi: true
        rosso1:

jbemmel · 2025-06-24T11:44:46Z

For the data model, I would consider the following:

ethernet_segments:
  seg1:
    id: lacp
  seg2:
    id: aa:bb:cc:dd:ee:ff:00:11:22:33

links:
- lag:
      members:
      - sw1a:
          vlan.access: rosso
          lag:
            lacp_system_id: 1
            esi: seg1
        rosso1:
      - sw1b:
          vlan.access: rosso
          lag:
            lacp_system_id: 1
            esi: seg1
        rosso1:

I'm not sure if this is best implemented as a separate module, or as part of the evpn module (since esi is closely linked with evpn, e.g. on SR Linux it would require evpn). The lacp_system_id obviously belongs with the lag module, but esi could be a separate interface attribute not necessarily under lag

ssasso · 2025-06-24T11:53:00Z

I'm not sure if this is best implemented as a separate module, or as part of the evpn module (since esi is closely linked with evpn, e.g. on SR Linux it would require evpn). The lacp_system_id obviously belongs with the lag module, but esi could be a separate interface attribute not necessarily under lag

But this is explicitly addressing ESI-LAG, where the esi attribute must stay under lag config (at least both in junos, frr, eos). And IIRC in FRR esi mh attributes are supported only under bond interfaces (while other vendors support esi also outside the lag).

However the big question is: how can we separate them but at the same time keep them tied in some cases? that is what I was not able to easily achieve with a plugin (especially on the "validation" of "mlag" - i.e., not requiring peerlink & co...).

jbemmel · 2025-06-24T12:12:09Z

I'm not sure if this is best implemented as a separate module, or as part of the evpn module (since esi is closely linked with evpn, e.g. on SR Linux it would require evpn). The lacp_system_id obviously belongs with the lag module, but esi could be a separate interface attribute not necessarily under lag

But this is explicitly addressing ESI-LAG, where the esi attribute must stay under lag config (at least both in junos, frr, eos). And IIRC in FRR esi mh attributes are supported only under bond interfaces (while other vendors support esi also outside the lag).

However the big question is: how can we separate them but at the same time keep them tied in some cases? that is what I was not able to easily achieve with a plugin.

{% for intf in interfaces if intf.type == 'lag' and intf.lag.esi is defined %}
interface {{ intf.ifname }}
{%   if intf.lag._esi_auto_derive|default(false) %}
    evpn mh es-id {{intf.lag._esi_manual}}
{%   else %}
    evpn mh es-id {{intf.lag.esi}}
{%   endif %}
!
{% endfor %}

Taking an example from your FRR config - this is evpn configuration, not lag. If FRR has the limitation that ESI can only be applied to lags, a device quirk could enforce that (or a feature flag inside the esi module/plugin, e.g. modes: [ lag, evpn ] where FRR might have modes: [ lag ] and SR Linux would have modes: [ evpn ] )

In my example, if id is set to lacp the ESI logic would check that the interface to which the ESI is applied is also part of a lag

My main concern, is that the data model imho shouldn't be based on the (limited) way in which some particular platform supports the feature. In my mind the feature is "ethernet segments", with implementations under EVPN and (here) LAG

ssasso · 2025-06-24T12:17:08Z

My main concern, is that the data model imho shouldn't be based on the (limited) way in which some particular platform supports the feature. In my mind the feature is "ethernet segments", with implementations under EVPN and (here) LAG

totally agree on this.

but how we can change the LAG validation?

i.e., if I apply an ethernet segment to a (m)lag interface, then in some way we need to disable mlag peer link validation (or device not supporting "strict" mlag) - so evpn and lag context will be mixed here.

jbemmel · 2025-06-24T12:46:07Z

but how we can change the LAG validation?

i.e., if I apply an ethernet segment to a (m)lag interface, then in some way we need to disable mlag peer link validation (or device not supporting "strict" mlag) - so evpn and lag context will be mixed here.

I can see how it's hard to get a "clean" separation / modularity here

I would say it'd be ok to modify the logic in the lag module to check for an 'esi' interface attribute (and perhaps a lag.mlag_esi feature flag), and skip the mlag-not-supported error in that case. However, the provisioning scripts and global ethernet_segments etc. would be better placed in a separate module/plugin imho

ssasso · 2025-06-25T10:28:42Z

@jbemmel I really need your help on this topic:

I moved the implementation to an external plugin, and I was successfully able to add additional verification to 'lag' module to avoid errors for not supporting mclag.

However, after passing throught the lag plugin, the vlan module starts complaining:

Topology snippet:

links:
# EVPN/VXLAN Link
- s1:
  s2:
  mtu: 1600
# ESI-LAG
- lag:
    members:
    - s1:
        lag.lacp_system_id: 1
        evpn.es: seg_1
      x1:
    - s2:
        lag.lacp_system_id: 1
        evpn.es: seg_1
      x1:
  vlan.access: red

Error:

stefano@donkey:~/GIT_H/netlab/tests/integration/evpn.es$ netlab create -p clab -d vjunos-switch 01-esi-lag.yml
[ERRORS]  Errors found in 01-esi-lag.yml
[VALUE]   vlan: You must use 'nodes.s1.vlans.red' dictionary to set VLAN interface attributes,
[DATA]    Link links[2], node s1, vlan red
          Interface VLAN forwarding mode: bridge, unexpected attributes evpn
          {'node': 's1', 'lag': {'lacp_system_id': 1, '_mlag': True, 'ifindex': 1}, 'evpn': {'es': 'seg_1'},
          'vlan': {'access': 'red'}, '_vlan_mode': 'bridge', 'ipv4': False, 'ipv6': False, 'type':
          'lag'}
[VALUE]   vlan: You must use 'nodes.s2.vlans.red' dictionary to set VLAN interface attributes,
[DATA]    Link links[2], node s2, vlan red
          Interface VLAN forwarding mode: bridge, unexpected attributes evpn
          {'node': 's2', 'lag': {'lacp_system_id': 1, '_mlag': True, 'ifindex': 1}, 'evpn': {'es': 'seg_1'},
          'vlan': {'access': 'red'}, '_vlan_mode': 'bridge', 'ipv4': False, 'ipv6': False, 'type':
          'lag'}
[FATAL]   Cannot proceed beyond this point due to errors, exiting

I guess this is related on how the lag module copies attributes to additional links? any hint?

How did you make vlan not complaining for the 'lag' attributes?

thanks :-)

ssasso · 2025-06-25T10:54:11Z

How did you make vlan not complaining for the 'lag' attributes?

nevermind, I think I got it. it's in the vlan module itself.

netsim/extra/evpn.es/plugin.py

netsim/data/types.py

netsim/extra/evpn.es/defaults.yml

netsim/modules/lag.yml

jbemmel · 2025-06-25T14:01:29Z

tests/integration/evpn.es/01-esi-lag.yml

+- lag:
+    members:
+    - s1:
+        lag.lacp_system_id: 1


As commented above, I think it would be great if the plugin could implement logic to auto-generate the lacp_system_id:
When a lag interface references an ES with 'auto: true', if there is no manual override generate a lacp_system_id based on the minimum node ID of all the nodes attached to that segment (could be >2)

jbemmel · 2025-06-25T14:03:10Z

tests/topology/expected/lag-l2.yml

      name: r1 -> r2
      neighbors:
      - ifname: port-channel1
+        lag:


It is odd that this changes - could point at a bug somewhere (not necessarily in your additions)

Agree -- this should be investigated. Also, this is definitely triggered by the additions (not saying the bug is not somewhere else), or we would have already seen it in dev branch.

It's triggered by the addition of the lacp_system_id interface attribute

Add

intf_to_neighbor: False # By default, do not include lag attributes in neighbors

to fix it

ssasso · 2025-06-25T16:10:08Z

Side note: frr seems to have some problems, under investigation.

Control plane looks good, looks like the official example found here: https://github.com/FRRouting/frr/tree/master/tests/topotests/bgp_evpn_mh

But test is not passing.

Included test is passing for vjunos-switch and eos.

ssasso · 2025-06-25T16:10:56Z

@ipspace I'd love to hear also your feedback on this plugin - but no rush.

jbemmel

(IMHO) ethernet segments should be similar to vlans, in that they always need to be declared - even if all IDs are auto-generated. It helps users avoid mistakes by enforcing consistency / referential integrity
should add intf_to_neighbor: False to lag module to avoid changes to neighbors

jbemmel · 2025-07-02T14:27:59Z

Note that Cumulus NVUE fails to pass the test; after disabling the link, the ping from h2 to h1 fails

jbemmel · 2025-07-02T18:02:20Z

tests/integration/evpn.multihoming/01-esi-lag.yml

+    description: End-to-end connectivity after a LAG member failure
+    nodes: [ h2, h3 ]
+    wait_msg: Waiting for ESI-LAG convergence
+    level: warning


Should this be a mere warning? Supporting failover scenario's is the whole point of doing ES Multihoming, Cumulus NVUE currently fails this test - so it basically doesn't support MH properly

I copied (and adapted) your M-LAG tests. If this should not be a warning here, it should not be a warning here as well:

netlab/tests/integration/lag/10-mlag.yml

Line 60 in 110bf8a

failover:

ssasso · 2025-07-03T07:06:28Z

Updated lag to avoid attribute propagation on neigh
@ipspace I decided to move the validation on a separate function as per your suggestion to keep the plugin flow "clean" as much as possible
@jbemmel will investigate further NVUE - but if you have some spare time to have a look at it as well, it's appreciated

ipspace · 2025-07-03T07:10:26Z

@ipspace I decided to move the validation on a separate function as per your suggestion to keep the plugin flow "clean" as much as possible

Thank you!

@jbemmel will investigate further NVUE - but if you have some spare time to have a look at it as well, it's appreciated

Based on how Nvidia rolls, NVUE is dead to me and I plan to declare it obsolete (together with CL 4.x) in the next release notes (did I mention https://www.youtube.com/shorts/tQIdxbWhHSM already?).

ipspace · 2025-07-07T17:38:43Z

I lost track 🤦‍♂️ Is this ready for another review or a merge?

ssasso · 2025-07-07T20:06:40Z

I lost track 🤦‍♂️ Is this ready for another review or a merge?

Rebased, ready for review and merge if you think it's ok.

I won't investigate further on cumulus convergence on link failure.

* Use 'data.get_empty_box' to create a box * There's no need to create dicts inside a box (they are created on first reference) * Dict can be used instead of a set to detect duplicates * data.append_to_list implements the 'append to list, create if missing' functionality

ipspace

Looks good to me (and EOS implementation works ;). I streamlined the _esi_stats code a bit; if you're OK with those changes, let's merge this one.

ssasso · 2025-07-08T15:37:16Z

Looks good to me (and EOS implementation works ;). I streamlined the _esi_stats code a bit; if you're OK with those changes, let's merge this one.

all good, you can proceed, thank you!!!

Repository owner deleted a comment from github-actions bot Jun 25, 2025