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

[voq/inbandif] Voq inbandif port #1363

Merged
merged 3 commits into from
Apr 9, 2021

Conversation

vganesan-nokia
Copy link
Contributor

@vganesan-nokia vganesan-nokia commented Jan 18, 2021

- What I did

Inband port can be made available in PORT table. But regular port handlngs are
not applicable for Inband port. This PR has change to avoid regular port handling for inband port

Ref: VOQ HLD https://github.com/Azure/SONiC/blob/master/doc/voq/voq_hld.md

- How I did it

The inband port is considered similar to back plane port. An inband_prefix = "Ethernet-IB" and this is checked in all places to avoid regular port handling.

- How to verify it

  • Expose a port with name prefixed with "Ethernet-IB" to SONiC via port_config.ini
  • Have an entry in the PORT table for this port with name prefixed with "Ethernet-IB"
  • When the system is booted, there should not be errors related to Inband interface

- 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)

Note: This PR depends on sonic-buildimage PR sonic-net/sonic-buildimage#6477

@anshuv-mfst
Copy link

@eswaran (Arista), @abdosi - could you please review the PR, Thanks.

ngoc-do
ngoc-do previously approved these changes Feb 8, 2021
@@ -322,7 +322,7 @@ class SFPShow(object):
sorted_table_keys = natsorted(port_table_keys)
for i in sorted_table_keys:
interface = re.split(':', i, maxsplit=1)[-1].strip()
if interface and interface.startswith(front_panel_prefix()) and not interface.startswith(backplane_prefix()):
if interface and interface.startswith(front_panel_prefix()) and not interface.startswith(backplane_prefix()) and not interface.startswith(inband_prefix()):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think right now, there is no need to check not interface.startswith(backplane_prefix()) and not interface.startswith(inband_prefix()) since front_panel_prefix is different from the other prefixes. But it's safer in case in future the prefixes are changed. So approved.

Copy link
Contributor Author

@vganesan-nokia vganesan-nokia Feb 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for approving. We changed the prefix. As per the review comments during Recycle port HLD, we changed the Inband port prefix to "Ethernet-IB". With this, it is required to handle inband_prefix similar to backplane_prefix.

abdosi
abdosi previously approved these changes Mar 7, 2021
Copy link
Contributor

@abdosi abdosi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@abdosi
Copy link
Contributor

abdosi commented Mar 7, 2021

/Azurepipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@anshuv-mfst
Copy link

@ngoc-do - please re-approve

@anshuv-mfst
Copy link

@ngoc-do, @eswaranb - please re-approve.

ngoc-do
ngoc-do previously approved these changes Mar 22, 2021
@abdosi
Copy link
Contributor

abdosi commented Apr 7, 2021

can we add unit test case to check port is skip.

@vganesan-nokia
Copy link
Contributor Author

can we add unit test case to check port is skip.

Yes. I'll add test case to check skipping Ethernet-IB.

vedganes added 2 commits April 7, 2021 16:25
Signed-off-by: vedganes <vedavinayagam.ganesan@nokia.com>

Inband port is avaialable in PORT table. But regular port handlings are
not applicable for Inband port. Changes in this PR are to avoid regular
port handling on Inband port.
Signed-off-by: vedganes <vedavinayagam.ganesan@nokia.com>

Changes to skip reporting error for host routes of voq neighbors
programmed on the inband interface. These routes will be in APPL DB but
will not be available in ASIC DB. We do not require them in ASIC DB
since host routes for the voq neighbors are created in SAI as part of
neighbor programming itself.
@vganesan-nokia
Copy link
Contributor Author

Capturing approvals before rebasing

image

- Unit test case added to cover code changes in route check untility to
avoid reporting error for missing routes in asic db for routes of voq
neighbors on inband interface
- Mock appl db is updated to include inband interface so that sfpshow code
changes for skipping inband interface will be unit tested

Signed-off-by: vedganes <vedavinayagam.ganesan@nokia.com>
@vganesan-nokia
Copy link
Contributor Author

can we add unit test case to check port is skip.

Yes. I'll add test case to check skipping Ethernet-IB.

Fixed. Added test case for route check to unit test changes done in route_check.py. Updated the mock appl db to include inband interface so that the code changes in sfpshow will be unit tested (the verification of skipping inband port).

@abdosi abdosi merged commit 02b263a into sonic-net:master Apr 9, 2021
@renukamanavalan
Copy link
Contributor

@abdosi, should this get into 202012 ?

gitsabari pushed a commit to gitsabari/sonic-utilities that referenced this pull request Jun 15, 2021
Inband port can be made available in PORT table. But regular port handlngs are
not applicable for Inband port. This PR has change to avoid regular port handling for inband port for route_check and sfpshow script.
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.

5 participants