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

[portchannel] Added ACL/PBH binding checks to the port before getting added to portchannel #2151

Merged
merged 9 commits into from May 18, 2022

Conversation

vivekrnv
Copy link
Contributor

@vivekrnv vivekrnv commented May 5, 2022

Signed-off-by: Vivek Reddy Karri vkarri@nvidia.com

What I did

The change to handle it in oA is already added.

When this check is not performed when adding the config, the portchannel configuration will be inconsistent b/w Kernel and ASIC

How I did it

Utilize the match infra to implement methods to check for ACL/PBH bindings to a port

How to verify it

Unit tests

Add Config
root@r-lionfish-16:/home/admin# config pbh table add NVGRE --interface-list "Ethernet0,Ethernet56" --description "Whatever"
root@r-lionfish-16:/home/admin# sudo config acl add table EVERFLOW MIRROR --ports "Ethernet236"
root@r-lionfish-16:/home/admin# config portchannel add PortChannel1111

Try adding members to portchannel
root@r-lionfish-16:/home/admin# config portchannel member add PortChannel1111 Ethernet0
Usage: config portchannel member add [OPTIONS] <portchannel_name> <port_name>
Try "config portchannel member add -h" for help.

Error: Port Ethernet0 is already bound to following PBH_TABLES: ['PBH_TABLE|NVGRE']

root@r-lionfish-16:/home/admin# config portchannel member add PortChannel1111 Ethernet236
Usage: config portchannel member add [OPTIONS] <portchannel_name> <port_name>
Try "config portchannel member add -h" for help.

Error: Port Ethernet236 is already bound to following ACL_TABLES: ['ACL_TABLE|EVERFLOW']

Check debug dump:
root@r-lionfish-16:/home/admin# dump state portchannel_member "PortChannel1111|Ethernet0,PortChannel1111|Ethernet236"
{
    "PortChannel1111|Ethernet0": {
        "CONFIG_DB": {
            "keys": [],
            "tables_not_found": [
                "PORTCHANNEL_MEMBER"
            ]
        },
        "APPL_DB": {
            "keys": [],
            "tables_not_found": [
                "LAG_MEMBER_TABLE"
            ]
        },
        "ASIC_DB": {
            "keys": [],
            "tables_not_found": [
                "ASIC_STATE:SAI_OBJECT_TYPE_LAG_MEMBER"
            ]
        },
        "STATE_DB": {
            "keys": [],
            "tables_not_found": [
                "LAG_MEMBER_TABLE"
            ]
        }
    },
    "PortChannel1111|Ethernet236": {
        "CONFIG_DB": {
            "keys": [],
            "tables_not_found": [
                "PORTCHANNEL_MEMBER"
            ]
        },
        "APPL_DB": {
            "keys": [],
            "tables_not_found": [
                "LAG_MEMBER_TABLE"
            ]
        },
        "ASIC_DB": {
            "keys": [],
            "tables_not_found": [
                "ASIC_STATE:SAI_OBJECT_TYPE_LAG_MEMBER"
            ]
        },
        "STATE_DB": {
            "keys": [],
            "tables_not_found": [
                "LAG_MEMBER_TABLE"
            ]
        }
    }
}
root@r-lionfish-16:/home/admin# dump state portchannel "PortChannel1111"
{
    "PortChannel1111": {
        "CONFIG_DB": {
            "keys": [
                {
                    "PORTCHANNEL|PortChannel1111": {
                        "admin_status": "up",
                        "lacp_key": "auto",
                        "min_links": "1",
                        "mtu": "9100"
                    }
                }
            ],
            "tables_not_found": []
        },
        "APPL_DB": {
            "keys": [
                {
                    "LAG_TABLE:PortChannel1111": {
                        "admin_status": "up",
                        "mtu": "9100",
                        "oper_status": "down"
                    }
                }
            ],
            "tables_not_found": []
        },
        "ASIC_DB": {
            "keys": [],
            "tables_not_found": [
                "ASIC_STATE:SAI_OBJECT_TYPE_LAG"
            ]
        },
        "STATE_DB": {
            "keys": [
                {
                    "LAG_TABLE|PortChannel1111": {
                        "admin_status": "up",
                        "mtu": "9100",
                        "oper_status": "down",
                        "runner.active": "true",
                        "runner.fallback": "false",
                        "runner.fast_rate": "false",
                        "setup.kernel_team_mode_name": "loadbalance",
                        "setup.pid": "154",
                        "state": "ok",
                        "team_device.ifinfo.dev_addr": "1c:34:da:1c:9f:00",
                        "team_device.ifinfo.ifindex": "136"
                    }
                }
            ],
            "tables_not_found": []
        }
    }
}

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

vivekrnv and others added 7 commits April 15, 2022 23:35
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@dgsudharsan dgsudharsan requested a review from nazariig May 6, 2022 00:08
dgsudharsan
dgsudharsan previously approved these changes May 6, 2022
@dprital dprital requested review from prsunny and qiluo-msft May 9, 2022 16:51
dgsudharsan
dgsudharsan previously approved these changes May 11, 2022
@liat-grozovik
Copy link
Collaborator

@nazariig please review

nazariig
nazariig previously approved these changes May 11, 2022
@liat-grozovik liat-grozovik changed the title [portchannel] Added ACl/PBH binding checks to the port before getting added to portchannel [portchannel] Added ACL/PBH binding checks to the port before getting added to portchannel May 11, 2022
@dprital
Copy link
Collaborator

dprital commented May 12, 2022

@qiluo-msft - can you please assist with review ?

utilities_common/db.py Outdated Show resolved Hide resolved
@vivekrnv vivekrnv dismissed stale reviews from nazariig and dgsudharsan via c36f15d May 17, 2022 18:36
@liat-grozovik liat-grozovik merged commit 29503ab into sonic-net:master May 18, 2022
dprital added a commit to dprital/sonic-buildimage that referenced this pull request May 25, 2022
Update sonic-utilities submodule pointer to include the following:
* [GCU] Handling type1 lists ([sonic-net#2171](sonic-net/sonic-utilities#2171))
* [yang] extend ConfigMgmt constructor to pass YANG options ([sonic-net#2118](sonic-net/sonic-utilities#2118))
* [dump] implement ACL modules ([sonic-net#2153](sonic-net/sonic-utilities#2153))
* show commands for SYSTEM READY ([sonic-net#1851](sonic-net/sonic-utilities#1851))
* [GCU] Handling non-compliant leaf-list with string values ([sonic-net#2174](sonic-net/sonic-utilities#2174))
* Add sonic-delayed.target to Application Extension .timer file generator ([sonic-net#2176](sonic-net/sonic-utilities#2176))
* [portconfig] Allow to configure interface mtu for physical ports ([#l](https://github.com/Azure/sonic-utilities/pull/l))
* Broadcast Unknown-multicast and Unknown-unicast Storm-control  ([sonic-net#928](sonic-net/sonic-utilities#928))
* sonic-utils: initial support for link-training ([sonic-net#2071](sonic-net/sonic-utilities#2071))
* [portchannel] Added ACL/PBH binding checks to the port before getting added to portchannel ([sonic-net#2151](sonic-net/sonic-utilities#2151))
* Modify override testcase to cover PORT admin_status ([sonic-net#2165](sonic-net/sonic-utilities#2165))
* [GCU] Validate peer_group_range ip_range are correct ([sonic-net#2145](sonic-net/sonic-utilities#2145))
* [auto-ts] add memory check ([sonic-net#2116](sonic-net/sonic-utilities#2116))
* support new interface types CR8/SR8/KR8/LR8 which are brougnt by SAI V.1.10.2 ([sonic-net#2167](sonic-net/sonic-utilities#2167))
* [scripts/fast-reboot] Add option to include ssd-upgrader-part boot option with SONiC partition ([sonic-net#2150](sonic-net/sonic-utilities#2150))
* [config reload] Fix invalid rstrip. ([sonic-net#2157](sonic-net/sonic-utilities#2157))
* Accept 0 for queue and dscp ([sonic-net#2162](sonic-net/sonic-utilities#2162))

Signed-off-by: dprital <drorp@nvidia.com>
@judyjoseph
Copy link
Contributor

Please raise a new PR for 202111 ,, as there are conflicts

@vivekrnv
Copy link
Contributor Author

Please raise a new PR for 202111 ,, as there are conflicts

Backport PR: #2186

liat-grozovik pushed a commit that referenced this pull request May 26, 2022
…e getting added to portchannel (#2186)

- What I did
Backport #2151 to 202111
The change to handle it in oA is already added.

When this check is not performed when adding the config, the portchannel configuration will be inconsistent b/w Kernel and ASIC

- How I did it
Utilize the match infra to implement methods to check for ACL/PBH bindings to a port

- How to verify it
Unit tests

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
dbarashinvd pushed a commit to dbarashinvd/sonic-utilities that referenced this pull request Jul 11, 2022
…e getting added to portchannel (sonic-net#2186)

- What I did
Backport sonic-net#2151 to 202111
The change to handle it in oA is already added.

When this check is not performed when adding the config, the portchannel configuration will be inconsistent b/w Kernel and ASIC

- How I did it
Utilize the match infra to implement methods to check for ACL/PBH bindings to a port

- How to verify it
Unit tests

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
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

Successfully merging this pull request may close these issues.

None yet

7 participants