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.

Sign up for GitHub

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

Jump to bottom

[dpb|config] Fix the validation logic of breakout mode #1440

Merged
merged 4 commits into from
Feb 23, 2021
Merged

[dpb|config] Fix the validation logic of breakout mode #1440

merged 4 commits into from
Feb 23, 2021

Conversation

samaity
Copy link
Contributor

@samaity samaity commented Feb 18, 2021 •
edited
Loading

Signed-off-by: Sangita Maity [email protected]

What I did

As per latest update in DPB DOC, fixed this bug

previously we had string value in "breakout_modes" key so it was not matching the whole string, But after the update via, now "breakout_modes" contain a dictionary where key is the breakout_mode and value is the alias. So we can easily check whether the key is present or not.

DEPENDENCY on Update Dynamic Port Breakout Logic for flexible alias support which fixes the issue

How I did it

checked whether the user-specified breakout mode key is present under "breakout_modes" inplatform.json.

How to verify it

If you only have 1x100G[50G,40G,25G,10G], the below CLI command should fail:
config interface breakout Ethernet96 1x100G as it matches the whole key.

admin@lnos-x1-a-csw05:~$ sudo config interface breakout Ethernet40
1x100G[40G]  2x50G        4x25G[10G]

admin@lnos-x1-a-csw05:~$ sudo config interface breakout Ethernet40 2x50G -v -y

Running Breakout Mode : 2x50G
Target Breakout Mode : 2x50G
[WARNING] No action will be taken as current and desired Breakout Mode are same.

it matches the whole key.

admin@lnos-x1-a-csw05:~$ sudo config interface breakout Ethernet40 4x25G -v -y
[ERROR] Target mode 4x25G is not available for the port Ethernet40
Aborted!

@samaity samaity mentioned this pull request Feb 18, 2021
Closed
4 tasks
@samaity samaity mentioned this pull request Feb 20, 2021
Merged
4 tasks
@samaity samaity marked this pull request as ready for review February 20, 2021 00:26
zhenggen-xu
zhenggen-xu previously approved these changes Feb 20, 2021
View reviewed changes
@lguohan
Copy link
Contributor

lguohan commented Feb 23, 2021

can you check why unit test is still failling?

lguohan
lguohan reviewed Feb 23, 2021
View reviewed changes
tests/config_dpb_test.py Outdated Show resolved Hide resolved
lguohan
lguohan reviewed Feb 23, 2021
View reviewed changes
tests/config_dpb_test.py Outdated Show resolved Hide resolved
lguohan
lguohan reviewed Feb 23, 2021
View reviewed changes
tests/config_dpb_test.py Outdated Show resolved Hide resolved
Sangita Maity and others added 4 commits February 23, 2021 10:48
@lguohan lguohan merged commit 787d884 into sonic-net:master Feb 23, 2021
This was referenced Feb 24, 2021
Closed
Closed
@jleveque jleveque mentioned this pull request Feb 25, 2021
Closed
lguohan pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Feb 26, 2021
[DPB|master] Update Dynamic Port Breakout Logic for flexible alias su…
18263c9 
…pport a… (#6831)

To fix [DPB| wrong aliases for interfaces](#6024) issue, implimented flexible alias support [design doc](sonic-net/SONiC#749)

> [[dpb|config] Fix the validation logic of breakout mode](sonic-net/sonic-utilities#1440) depends on this

#### How I did it

1. Removed `"alias_at_lanes"` from port-configuration file(i.e. platfrom.json) 
2. Added dictionary to "breakout_modes" values. This defines the breakout modes available on the platform for this parent port, and it maps to the alias list. The alias list presents the alias names for individual ports in order under this breakout mode.
```
{
    "interfaces": {
        "Ethernet0": {
            "index": "1,1,1,1",
            "lanes": "0,1,2,3",
            "breakout_modes": {
                "1x100G[40G]": ["Eth1"],
                "2x50G": ["Eth1/1", "Eth1/2"],
                "4x25G[10G]": ["Eth1/1", "Eth1/2", "Eth1/3", "Eth1/4"],
                "2x25G(2)+1x50G(2)": ["Eth1/1", "Eth1/2", "Eth1/3"],
                "1x50G(2)+2x25G(2)": ["Eth1/1", "Eth1/2", "Eth1/3"]
            }
        }
}
```
#### How to verify it
`config interface breakout`

Signed-off-by: Sangita Maity <[email protected]>
anand-kumar-subramanian pushed a commit to anand-kumar-subramanian/sonic-utilities that referenced this pull request Mar 2, 2021
@lguohan @anand-kumar-subramanian
[dpb|config] Fix the validation logic of breakout mode (sonic-net#1440)
15c2a68 
As per latest update in DPB DOC, fixed this bug

previously we had string value in "breakout_modes" key so it was not matching the whole string, But after the update via, now "breakout_modes" contain a dictionary where key is the breakout_mode and value is the alias. So we can easily check whether the key is present or not.

Signed-off-by: Sangita Maity <[email protected]>
Co-authored-by: Guohan Lu <[email protected]>
praveen-li pushed a commit to praveen-li/sonic-utilities that referenced this pull request Jun 8, 2021
@lguohan
[dpb|config] Fix the validation logic of breakout mode (sonic-net#1440)
3ae87e2 
As per latest update in DPB DOC, fixed this bug

previously we had string value in "breakout_modes" key so it was not matching the whole string, But after the update via, now "breakout_modes" contain a dictionary where key is the breakout_mode and value is the alias. So we can easily check whether the key is present or not.

Signed-off-by: Sangita Maity <[email protected]>
Co-authored-by: Guohan Lu <[email protected]>
praveen-li pushed a commit to praveen-li/sonic-buildimage that referenced this pull request Jun 8, 2021
[DPB|master] Update Dynamic Port Breakout Logic for flexible alias su…
4de9c56 
…pport a… (sonic-net#6831)

To fix [DPB| wrong aliases for interfaces](sonic-net#6024) issue, implimented flexible alias support [design doc](sonic-net/SONiC#749)

> [[dpb|config] Fix the validation logic of breakout mode](sonic-net/sonic-utilities#1440) depends on this

1. Removed `"alias_at_lanes"` from port-configuration file(i.e. platfrom.json)
2. Added dictionary to "breakout_modes" values. This defines the breakout modes available on the platform for this parent port, and it maps to the alias list. The alias list presents the alias names for individual ports in order under this breakout mode.
```
{
    "interfaces": {
        "Ethernet0": {
            "index": "1,1,1,1",
            "lanes": "0,1,2,3",
            "breakout_modes": {
                "1x100G[40G]": ["Eth1"],
                "2x50G": ["Eth1/1", "Eth1/2"],
                "4x25G[10G]": ["Eth1/1", "Eth1/2", "Eth1/3", "Eth1/4"],
                "2x25G(2)+1x50G(2)": ["Eth1/1", "Eth1/2", "Eth1/3"],
                "1x50G(2)+2x25G(2)": ["Eth1/1", "Eth1/2", "Eth1/3"]
            }
        }
}
```
`config interface breakout`

Signed-off-by: Sangita Maity <[email protected]>
	device/mellanox/x86_64-mlnx_msn2700-r0/platform.json
praveen-li pushed a commit to praveen-li/sonic-buildimage that referenced this pull request Jun 25, 2021
[DPB|master] Update Dynamic Port Breakout Logic for flexible alias su…
e0ac130 
…pport a… (sonic-net#6831)

To fix [DPB| wrong aliases for interfaces](sonic-net#6024) issue, implimented flexible alias support [design doc](sonic-net/SONiC#749)

> [[dpb|config] Fix the validation logic of breakout mode](sonic-net/sonic-utilities#1440) depends on this

1. Removed `"alias_at_lanes"` from port-configuration file(i.e. platfrom.json)
2. Added dictionary to "breakout_modes" values. This defines the breakout modes available on the platform for this parent port, and it maps to the alias list. The alias list presents the alias names for individual ports in order under this breakout mode.
```
{
    "interfaces": {
        "Ethernet0": {
            "index": "1,1,1,1",
            "lanes": "0,1,2,3",
            "breakout_modes": {
                "1x100G[40G]": ["Eth1"],
                "2x50G": ["Eth1/1", "Eth1/2"],
                "4x25G[10G]": ["Eth1/1", "Eth1/2", "Eth1/3", "Eth1/4"],
                "2x25G(2)+1x50G(2)": ["Eth1/1", "Eth1/2", "Eth1/3"],
                "1x50G(2)+2x25G(2)": ["Eth1/1", "Eth1/2", "Eth1/3"]
            }
        }
}
```
`config interface breakout`

Signed-off-by: Sangita Maity <[email protected]>
	device/mellanox/x86_64-mlnx_msn2700-r0/platform.json
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
[DPB|master] Update Dynamic Port Breakout Logic for flexible alias su…
b0c3072 
…pport a… (sonic-net#6831)

To fix [DPB| wrong aliases for interfaces](sonic-net#6024) issue, implimented flexible alias support [design doc](sonic-net/SONiC#749)

> [[dpb|config] Fix the validation logic of breakout mode](sonic-net/sonic-utilities#1440) depends on this

#### How I did it

1. Removed `"alias_at_lanes"` from port-configuration file(i.e. platfrom.json) 
2. Added dictionary to "breakout_modes" values. This defines the breakout modes available on the platform for this parent port, and it maps to the alias list. The alias list presents the alias names for individual ports in order under this breakout mode.
```
{
    "interfaces": {
        "Ethernet0": {
            "index": "1,1,1,1",
            "lanes": "0,1,2,3",
            "breakout_modes": {
                "1x100G[40G]": ["Eth1"],
                "2x50G": ["Eth1/1", "Eth1/2"],
                "4x25G[10G]": ["Eth1/1", "Eth1/2", "Eth1/3", "Eth1/4"],
                "2x25G(2)+1x50G(2)": ["Eth1/1", "Eth1/2", "Eth1/3"],
                "1x50G(2)+2x25G(2)": ["Eth1/1", "Eth1/2", "Eth1/3"]
            }
        }
}
```
#### How to verify it
`config interface breakout`

Signed-off-by: Sangita Maity <[email protected]>
@chiourung chiourung mentioned this pull request Oct 19, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhenggen-xu zhenggen-xu zhenggen-xu left review comments

@lguohan lguohan lguohan approved these changes

Assignees
No one assigned
Labels
Port Breakout 🔱
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@samaity @lguohan @zhenggen-xu