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] Support for inband port as regular port #6477

Merged
merged 4 commits into from
Apr 1, 2021

Conversation

vganesan-nokia
Copy link
Contributor

- Why I did it

Inband port can be made available in PORT table. But regular port handlngs are
not applicable for Inband port.

Changes in this PR are to make LLDP to consider Inband port and to avoid regular
port handling on Inband port.

Currently the Inband interface is used for VOQ chassis systems for asic to asic communications.
Ref: VOQ HLD https://github.com/Azure/SONiC/blob/master/doc/voq/voq_hld.md

- How I did it

  • Changes done in supervisor conf for LLDP to include Inband in the interface list
  • Changes done in platform common files to handle Inand port similar to how backplane port is handled

- How to verify it

  • Expose a port with name prefixed with "Inband" to SONiC via port_config.ini
  • Have an entry in the PORT table for this port with name "Inband"
  • When the system is booted, there should not be errors related to Inband interface
  • test_virtual_chassis.py which configures a front panel port as inband interface pass

- Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@anshuv-mfst
Copy link

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

@anshuv-mfst
Copy link

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

@@ -94,6 +94,11 @@ class LldpManager(daemon_base.DaemonBase):
"""
port_desc = None

# Skip port name prefixed with "Inband". These are recycle ports exposed in PORT_TABLE for
# asic-to-asic communication in VOQ based chassis system. We do not configure LLDP on these.
if 'Inband' in port_name:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be port_name.startswith('Inband') to match only on prefix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As per Recycle port HLD PR discussion, it was recommended to use name starting with Ethernet. For inband interface, it will be Ethernet-Inband0. For recirc port it will be Ethernet-Recirc0. I'll change this to port_name.startswith("Ethernet-Inband") or use the inband_prefix() as recommended in next comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@@ -94,6 +94,11 @@ class LldpManager(daemon_base.DaemonBase):
"""
port_desc = None

# Skip port name prefixed with "Inband". These are recycle ports exposed in PORT_TABLE for
# asic-to-asic communication in VOQ based chassis system. We do not configure LLDP on these.
if 'Inband' in port_name:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use inband_prefix() from sonic_py_common/interface.py?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I'll fix this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

vedganes added 4 commits February 17, 2021 14:24
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 are to make lldp to consider Inband port and to avoid regular
port handling on Inband port.
Signed-off-by: vedganes <vedavinayagam.ganesan@nokia.com>

Fixed code review comments
Signed-off-by: vedganes <vedavinayagam.ganesan@nokia.com>

Changes to avoid LLDP config on Inband interface. Since port type inband
interfaces are recycle ports (or may normal ports) used for asic-to-asic
communication, LLDP is not configured on these interfaces.
Signed-off-by: vedganes <vedavinayagam.ganesan@nokia.com>

(1) Based on review comments Recyle port HLD
(sonic-net/SONiC#742) the inband port name prefix is
changed from Inband to "Ethernet-IB" similar to what we have for
Ethernet-BP (Ethernet-Backplane) in multi-asic design. Changes are done
in interface.py to handle this changed name
(2) Code review comments fix for lldpmgrd
@vganesan-nokia
Copy link
Contributor Author

/Azurepipelines run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 6477 in repo Azure/sonic-buildimage

@abdosi
Copy link
Contributor

abdosi commented Mar 7, 2021

/Azurepipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vganesan-nokia
Copy link
Contributor Author

@abdosi, thanks for starting the Azurepipeline runs. Would you please complete review and approve so that we can merge? The PRs sonic-net/sonic-platform-common#159, sonic-net/sonic-platform-daemons#145 and sonic-net/sonic-utilities#1363 which you have already approved depend on this PR for build success.

@abdosi abdosi merged commit 973affc into sonic-net:master Apr 1, 2021
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-buildimage that referenced this pull request May 23, 2021
Changes in this PR are to make LLDP to consider Inband port and to avoid regular
port handling on Inband port.
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
Changes in this PR are to make LLDP to consider Inband port and to avoid regular
port handling on Inband port.
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.

4 participants