Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect/mismatched validation logic for mclag id #21339

Open
FireStormOOO opened this issue Jan 7, 2025 · 1 comment
Open

Incorrect/mismatched validation logic for mclag id #21339

FireStormOOO opened this issue Jan 7, 2025 · 1 comment

Comments

@FireStormOOO
Copy link

After creating an mclag configuration with id > 4095, unrelated commands are unable to complete due to a faulty or inconsistent validation rule.

The commands to create a new mclag definition allow a 16 bit id, and the feature appears to work correctly with higher id's (e.g. 8xxx). After configuring such an mclag, libyang fails to validate the config, due to seemingly faulty constraints clamping the value to a 12 bit number, resulting in some commands failing to execute:

sudo config interface breakout Ethernet124 4x25G[10G] 
Do you want to Breakout the port, continue? [y/N]: y

Running Breakout Mode : 1x100G[40G] 
Target Breakout Mode : 4x25G[10G]

Ports to be deleted : 
 {
    "Ethernet124": "100000"
}
Ports to be added : 
 {
    "Ethernet124": "25000",
    "Ethernet125": "25000",
    "Ethernet126": "25000",
    "Ethernet127": "25000"
}
libyang[0]: Value "8415" does not satisfy the constraint "1..4095" (range, length, or pattern). (path: /sonic-mclag:sonic-mclag/MCLAG_DOMAIN/MCLAG_DOMAIN_LIST[domain_id='8415']/domain_id)
libyang[0]: MCLAG Domain ID out of range (path: /sonic-mclag:sonic-mclag/MCLAG_DOMAIN/MCLAG_DOMAIN_LIST[domain_id='8415']/domain_id)
sonic_yang(3):Data Loading Failed:MCLAG Domain ID out of range
Data Loading Failed
MCLAG Domain ID out of range
ConfigMgmt Class creation failed
Failed to break out Port. Error: Failed to load the config. Error: ConfigMgmtDPB Class creation failed

*Note that Ethernet124 is not involved or referenced in any way with the MCLAG

    "MCLAG_DOMAIN": {
        "8415": {
            "peer_ip": "192.168.254.6",
            "peer_link": "PortChannel1",
            "source_ip": "192.168.254.5"
        }
    }

Steps to reproduce the issue:

  1. Create an MCLAG, as described in the layer 2 scenario, with the id > 4096, e.g. 8xxx. Make sure it works normally, passes traffic, etc. Of note, the documentation has only a mild warning against putting the keepalive traffic over the peerlink, but I found separate links to be a hard requirement; I currently have keepalive over the management interface.
  2. Attempt to breakout a port

Describe the results you received:

Error message above

Describe the results you expected:

Breakout command completes successfully and/or complains about something relevant.

Output of show version:

show version

SONiC Software Version: SONiC.202411.0-2ad469032
SONiC OS Version: 12
Distribution: Debian 12.8
Kernel: 6.1.0-22-2-amd64
Build commit: 2ad469032
Build date: Mon Dec 23 09:57:47 UTC 2024
Built by: <custom local build to enable Broadcom MCLAG>

Platform: x86_64-cel_seastone-r0
HwSKU: Seastone-DX010
ASIC: broadcom
ASIC Count: 1
Serial Number: DX010B2F108423LK100057
Model Number: R0872-F0010-01
Hardware Revision: N/A
Uptime: 03:25:37 up 10 days, 20:52,  1 user,  load average: 0.96, 1.04, 1.08
Date: Tue 07 Jan 2025 03:25:37

Additional information you deem important (e.g. issue happens only occasionally):

My first thought on seeing a 12 bit limit for the MCLAG id is something clever with VLAN tagging, but I don't see anything like that in the design docs or config.

Will report back and confirm this works normally once I drop the ID under 4096

@FireStormOOO
Copy link
Author

FireStormOOO commented Jan 7, 2025

I ended up locally patching 2 problems with the yang models:

Incorrect range src/sonic-yang-models/yang-models/sonic-mclag.yang, line 57

Incorrect constraint on the management interface requires every management IP to be paired with a default gateway, which is... not how interfaces work.
I struck the constraints on lines 44 and 49, and made gwaddr a union type with empty. The type constraints alone seem sufficient here, though ideally I'd also check that gwaddr, if present, is in the interface's subnet.

I also had to add the missing MGMT_PORT block as described here

Maybe config save should be warning if it writes a config that doesn't validate? That seems like it should always be generating an issue (if it wasn't valid, how did it make it into the running DB?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant